Skip to content
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

Merged
merged 1 commit into from
Dec 18, 2018
Merged

Add configuration files for various static analyzers #332

merged 1 commit into from
Dec 18, 2018

Conversation

jamiesnape
Copy link
Contributor

@jamiesnape jamiesnape commented Sep 4, 2018

Part 2 of CI-related work. This assumes you choose some derivative of the Google C++ Style Guide in #329.


This change is Reviewable

@SeanCurtis-TRI
Copy link
Contributor

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.

@jamiesnape
Copy link
Contributor Author

jamiesnape commented Sep 4, 2018

I am assuming most of these are going to wait for a while. No rush.

@sherm1
Copy link
Member

sherm1 commented Dec 13, 2018

@SeanCurtis-TRI can we move ahead with this yet?
@jamiesnape (needs a manual merge/rebase)

@jamiesnape
Copy link
Contributor Author

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.

Copy link
Member

@sherm1 sherm1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jamiesnape :lgtm:. 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

Copy link
Contributor Author

@jamiesnape jamiesnape left a 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.

Copy link
Member

@sherm1 sherm1 left a 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: :shipit: complete! all files reviewed, all discussions resolved

@sherm1 sherm1 merged commit 7f85d6a into flexible-collision-library:master Dec 18, 2018
@jamiesnape jamiesnape deleted the ci-2 branch December 18, 2018 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants