-
Notifications
You must be signed in to change notification settings - Fork 417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add configuration files for various static analyzers #332
Conversation
We might want to stall slightly on this. We've got an outstanding bug (apparently) with the EPA implementation. It would make sense to patch that, tag and release a new version of fcl before we start playing with the infrastructure (i.e., #329 related stuff). @hongkai-dai how credible do we feel the EPA bug is? Last we discussed it, it felt like we don't have a way to reproduce it. If that's the case, we can move on with a new release. Otherwise, we should probably move to release FCL so we can start tackling #329. |
I am assuming most of these are going to wait for a while. No rush. |
@SeanCurtis-TRI can we move ahead with this yet? |
Actually running the tools and fixing the defects is, I imagine, going to be after the next release, but I would assert that this is fine to review and potentially merge. I would also say this is really a starting point for the configurations to have something with which to develop CI based on my best guess for the project's requirements. If the configurations need to change now or prior to them being enforced, it is fine by me. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @jamiesnape . A couple of minor comments below.
Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jamiesnape)
CPPLINT.cfg, line 34 at r1 (raw file):
set noparent extensions=cpp,h
minor: should add cc
here as well (in case we go full-GSG)
CPPLINT.cfg, line 35 at r1 (raw file):
extensions=cpp,h filter=-build/c++11
BTW I think we might as well go to at least c++14
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on @sherm1)
CPPLINT.cfg, line 34 at r1 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
minor: should add
cc
here as well (in case we go full-GSG)
Done.
CPPLINT.cfg, line 35 at r1 (raw file):
Previously, sherm1 (Michael Sherman) wrote…
BTW I think we might as well go to at least c++14
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved
Part 2 of CI-related work. This assumes you choose some derivative of the Google C++ Style Guide in #329.
This change is