Isis3: What style should ISIS code follow?

[scsides]

Description

Should ISIS code follow a style, and if so, what style?

Currently all ISIS code is supposed to follow the style guide documented at: https://isis.astrogeology.usgs.gov/documents/CodingStandards/CodingStandards.html

There are good and bad things about having style guides. This issue is here to get a discussion started.

[kberryUSGS]

Here are some examples of various C++ coding styles followed by other projects as applied to usgscsm (_not_ ISIS.)

Links are diffs between the result of running clang-tidy with the listed formatter and the "current usgscsm dev" which was updated to use the Google format.

Google Style: https://github.com/USGS-Astrogeology/usgscsm/pull/307/files (special case: this one diffs against the original non-standardized code)
LLVM: https://github.com/USGS-Astrogeology/usgscsm/compare/dev...kberryUSGS:llvm
GNU: https://github.com/USGS-Astrogeology/usgscsm/compare/dev...kberryUSGS:gnu
Mozilla: https://github.com/USGS-Astrogeology/usgscsm/compare/dev...kberryUSGS:mozilla
Chromium: https://github.com/USGS-Astrogeology/usgscsm/compare/dev...kberryUSGS:chromium
WebKit: https://github.com/USGS-Astrogeology/usgscsm/compare/dev...kberryUSGS:webkit

Internal-only, these same examples are also available under:
/work/projects/isis/style_examples

I could easily do this for ISIS as well. Is anyone interested in seeing this?

Edit: Additional context: @scsides asked me to post these.

[rbeyer]

@scsides this is a great question.

I think ISIS should have such a thing.

There should be no expectation that the codebase should turn on a dime and instantly transform, but instead once we settle on a new version, then enforce as new code is committed to the repo.

The document you link contains information about a couple of different categories of "style." The Format category could be programmatically checked with a linter (like the examples @kberryUSGS showed). I don't have any recommendations, but it shouldn't be too difficult to settle on one, and it could even be integrated into the PR process.

Most of the other topics in that document are relevant to ISIS development and boil down to architecture decisions or required practices, and may be more difficult to enforce automatically (but that's what the PR/review process is for).

I think the document that you list would probably be a good place to start at (since that is theoretically where the code is "now" anyway). Maybe we should just bring it into the repo, slap a "draft" label on it, and work it with Issues and PRs until we're happy with it what it says?

[jessemapel]

I like the idea of moving the style guide off of the website and into the repo or wherever the dev docs go. I don't have a preference for the formatting part of the style guide. I would keep the ISIS naming conventions from our current style guide. I would update the application naming standard for the new application style. I would also drop the part about std::string vs qstring. We probably want to move away from QT classes in the API and start moving towards standard library classes.

[michaelaye]

i'm a big fan of automatically applied styles through linting/formatters.
It matters less, IMHO, what style one settles on than to have one at all that will be automatically and reliably enforced.
Having a format applied throughout a code base helps tremendously in reading and fast browsing it, I'm convinced that once the grumbling of the ones that voted for "the other obviously better style", productivity is enhanced all around.

[jlaura]

I ran across a free restyler at https://restyled.io. I see that they support two different C++ stylers: astyle and clang format. The restyler is free to use for OS projects in public repos on GitHub. If some of those two stylers looks like it has potential, I would be happy to get an ASC repo wired up.