Add a Linter action to the GitHub workflow

[jhiemstrawisc]

This commit adds a linter action that can automatically lint code in the repo. It's currently configured to analyze and automatically lint code whenever a PR is merged with master. The goal is to establish a cleaner and more structured code base. After the initial lint, the linter should only touch new/modified files.

[jhiemstrawisc]

For an example of what the linter is doing to the code, you can look at how it modified the contents of /src on a test branch I made. The only difference is I set the test up to lint on push to the branch instead of on PR to master.

https://github.com/jhiemstrawisc/scitokens-cpp/tree/linter_test

[djw8605]

Wow at all the changes. So, have we completely been writing the C++ code wrong?

[jhiemstrawisc]

Some of the choices made by clang-format remain a mystery to me... That being said, there are different style options that can be passed to clang-format in the action. The docs say that a few pre-set C++ style guide options include LLVM (default), GNU, Google, Chromium, Microsoft, Mozilla, and WebKit. If there are any sections of code that you don't like the formatting for, I can look to see how it changes with another style guide. Otherwise a user-specified style guide can be implemented in a .clang-format config file, but I'm not sure that's the first thing anyone wants to try.

[bbockelm]

@jhiemstrawisc - I wouldn’t mind switching to a style with 4-space indent. Seems most of the changes are coming from a switch to a 2-space indent.

[jhiemstrawisc]

@bbockelm Okay, I added a config file that should overwrite the LLVM style guide's default 2-space indents to use 4-space indents. Changes are reflected in the linter_test branch on my fork (see link above).

[bbockelm]

@jhiemstrawisc - hard to tell from the GH UI, how much does this decrease the overall changed lines?

[djw8605]

Indeed, could you make a new branch with just the new changes?

Also, I note that the linter will auto-commit changes back to the repo. Is there an option to simply comment on bad practices in github? I've seen linters that leave single line comments to lines that should be changed. We would want a massive re-lint of the repo, then do the single line comments.

[jhiemstrawisc]

There's a new test branch that shows changes the linter with 4-space indents will make to master located here.

As for having the linter comment on code practices instead of changing them, that should doable. I think the way to accomplish an initial lint of the entire repo and then switch over to have the linter only make suggestions would be to merge the PR with the action's auto_fix option set to true and then to switch the option to false in another push. When auto_fix is turned off, the action raises warnings about the malformed code in the form of commit annotations.

It looks like there's also something going on with the github_token permissions because the PR is coming from a fork branch instead of from from an internal repo branch. I'm looking into that to see if I can figure out what's going on. GitHub is suggesting running the action only on pull_request_target, but my sleuthing has found that combining pull_request_target with a checkout action is a good way to let other people exfiltrate github secrets just by issuing their own PR.

[djw8605]

I like the idea of committing auto-fix to true at first, then false after that. That should be easy.

With auto-fix = false, I wonder if it doesn't need a github token?

[bbockelm]

@djw8605 - why not auto-commit to the PR branch?

The way I see it, this would save the developer the work of copy/pasting between GH and the dev environment. If they don’t like how the branch looks with a bunch of messy white space commits, they can always rebase.