www.libssh2.org | Daily snapshots | Mailing list archive | Docs | Examples | github

Archive Index This month's Index

Subject: Code style and project status

Code style and project status

From: Daniel Stenberg <daniel_at_haxx.se>
Date: Sun, 17 Mar 2019 17:56:38 +0100 (CET)

Hi all,

I've kept away from the project for a good while, mostly idling in the
background as I've been determined to step down as maintainer completely.

I'm back here now primarily to put together and push out a new release
together with Will Cosgrove.

I'm slightly disappointed in the current shape of the project and of code I've
seen landed and that's one of the reasons why it now takes a lot more time
than anticipated to get the release done.

Some of the issues I found:

  - numerous compiler warnings with picky options have been introduced
  - no longer C89 compliant (//-comments and more)
  - TABs in the code
  - trailing whitespace all over
  - weird (inconsistent) code style used
  - more or less constant appveyor CI build failures
  - occasional VERY long source lines

I've put in efforts to clean some things up and have landed:

  - Removed all compiler warnings a picky gcc shows

  - I added a travis CI build that uses "configure --enable-debug" to
    trigger more compiler warnings and make it harder to land bad code.

  - I added an --enable-werror option that sets -Werror in the build so
    that it will FAIL on any warning in the build (including examples),
    now used by the travis job.

Possibly more controversial, what I want to land next is in PR 324 (
https://github.com/libssh2/libssh2/pull/324)

  - A code style checker job to the CI that will warn on basic code style
    violations, using the checksrc tool from the curl project.

  - It should cause the CI to fail on blatant style violations - it checks
    some of the most obvious things - but can still be foooled. It's not a
    replacement for human reviews. But as long as it warns on something,
    the code isn't code-style compliant.

  - A (rather large) code overhaul that unifies the style, white space,
    bracing, line lengths and some more to make sure that the new CI build
    still builds greeen.

  - The idea being that with (much) stricter tests and tooling, we will land
    more unified code and there will be less need for humans to point out the
    most obvious style violations in PRs.

Thoughts?

I realize I come here barging in, but I felt this was needed. I can be told
I'm wrong and I certainly think we could discuss code style etc if that's what
anyone wants. Especially I think the ones who actually write code in the
project more frequently than I do should have a say in how to write it.

I'm not married to a particular style but I will insist on the style to be
consistent *and enforced*.

-- 
  / daniel.haxx.se
_______________________________________________
libssh2-devel https://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2019-03-17

the libssh2 team