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.

Sign up for GitHub

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

Jump to bottom

Updated to VERA++ 1.3.0 in Travis and changed files to comply with style checks #884

Merged
merged 7 commits into from
Mar 5, 2018
Merged

Updated to VERA++ 1.3.0 in Travis and changed files to comply with style checks #884

merged 7 commits into from
Mar 5, 2018

Conversation

hakonsbm
Copy link
Contributor

In PR #691 all files were made compliant with style checks. However, because of a bug in VERA++ 1.2.1, a #pragma omp for directive is seen as a standard for loop. This was a problem in the file connection_manager.cpp, and VERA++ was therefore kept ignored in the static code analysis. Because of this, some new style violations were introduced by other PRs.

This PR

  • corrects the style violations
  • In Travis, switches from installing a packaged VERA++ 1.2.1 to installing VERA++ 1.3.0 from source (as this version isn't available as a package)
  • makes the static code analysis not ignore VERA++

@terhorstd terhorstd added ZC: Infrastructure DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Jan 22, 2018
@terhorstd terhorstd assigned gtrensch and unassigned gtrensch Jan 22, 2018
tammoippen
tammoippen reviewed Jan 22, 2018
View reviewed changes
build.sh Outdated
@@ -101,9 +101,15 @@ if [ "$xSTATIC_ANALYSIS" = "1" ] ; then
echo "+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +"

echo "MSGBLD0010: Initializing VERA++ static code analysis."
git clone https://github.com/verateam/vera.git
Copy link
Contributor

Choose a reason for hiding this comment

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

Vera++ is actually hosted at bitbucket: http://bitbucket.org/verateam/vera . The GitHub version is a clone. Further, cloning from that repo will give you whatever version is at branch master latest. This is not version 1.3.0 (which was released 2015-01-22). The last commit on master is also not stable. You should use the sources from the 1.3.0 download page: https://bitbucket.org/verateam/vera/wiki/Installation

gtrensch
gtrensch approved these changes Jan 24, 2018
View reviewed changes
Copy link
Contributor

@gtrensch gtrensch left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. All fine from my side.

@hakonsbm
Copy link
Contributor Author

@tammoippen I have made the changes you requested.

tammoippen
tammoippen approved these changes Jan 29, 2018
View reviewed changes
Copy link
Contributor

@tammoippen tammoippen left a comment

Choose a reason for hiding this comment

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

Looks good. 🎉

@janhahne
Copy link
Contributor

janhahne commented Feb 7, 2018

@hakonsbm The merged PR #858 changed some of the code of rate_neuron_ipn_impl.h and (unfortunately) also introduced some more missing {} blocks in rate_transformer_node.h and rate_transformer_node_impl.h. I was not aware of the problem, otherwise I would have added the blocks myself in #858, sorry about that!

@hakonsbm
Copy link
Contributor Author

hakonsbm commented Feb 7, 2018

@janhahne No problem, I'll take care of it.

@terhorstd terhorstd added this to the NEST 2.16 milestone Mar 5, 2018
@terhorstd terhorstd merged commit 9ec1ca9 into nest:master Mar 5, 2018
@hakonsbm hakonsbm deleted the vera_fixes_update branch April 3, 2018 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtrensch gtrensch gtrensch approved these changes

@tammoippen tammoippen tammoippen approved these changes

Assignees
No one assigned
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. ZC: Infrastructure DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Projects
None yet
Milestone
NEST 2.16
Development

Successfully merging this pull request may close these issues.
5 participants
@hakonsbm @janhahne @tammoippen @gtrensch @terhorstd