Skip to content

Make cpplint check actually work in CI #5929

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
Merged
merged 4 commits into from
Mar 21, 2021
Merged

Make cpplint check actually work in CI #5929

merged 4 commits into from
Mar 21, 2021

Conversation

tautschnig
Copy link
Collaborator

@tautschnig tautschnig commented Mar 15, 2021 •
edited
Loading

The Python script silently failed when invoked from run_diff.sh, which
redirects stdout to a file, when unidiff was not available.

Leaving in Draft state while debugging seeming silent failures of the cpplint action.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig force-pushed the fix-cpplint-action branch 3 times, most recently from 87aa410 to 3c14f05 Compare March 15, 2021 11:35
@tautschnig tautschnig mentioned this pull request Mar 15, 2021
Merged
2 tasks
@tautschnig tautschnig force-pushed the fix-cpplint-action branch 3 times, most recently from efc7cb5 to 9ae6596 Compare March 15, 2021 12:00
@tautschnig tautschnig marked this pull request as ready for review March 15, 2021 12:00
@tautschnig tautschnig requested a review from a team as a code owner March 15, 2021 12:00
@tautschnig
Copy link
Collaborator Author

check-cpplint is now working again, as can be seen in https://github.com/diffblue/cbmc/pull/5929/checks?check_run_id=2112286038.

@codecov
Copy link

codecov bot commented Mar 15, 2021 •
edited
Loading

Codecov Report

Merging #5929 (a5b3008) into develop (4c58926) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5929   +/-   ##
========================================
  Coverage    74.23%   74.23%           
========================================
  Files         1431     1431           
  Lines       155291   155291           
========================================
  Hits        115287   115287           
  Misses       40004    40004

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29a323f...a5b3008. Read the comment docs.

@tautschnig tautschnig changed the title diff_to_added_lines: report all errors on stderr Make cpplint check actually work in CI Mar 15, 2021
@tautschnig
Copy link
Collaborator Author

Regression tests are now also enabled.

@tautschnig tautschnig self-assigned this Mar 15, 2021
NlightNFotis
NlightNFotis reviewed Mar 16, 2021
View reviewed changes
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

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

Two questions from me.

@tautschnig tautschnig force-pushed the fix-cpplint-action branch 5 times, most recently from 7ff19f6 to 14f1c99 Compare March 18, 2021 00:54
@tautschnig
diff_to_added_lines: report all errors on stderr
e3aabfa 
The Python script silently failed when invoked from run_diff.sh, which
redirects stdout to a file, when unidiff was not available.
@tautschnig
cpplint strictly requires Python 2
585b359 
Using Python 3 (as is the default GitHub's Ubuntu 20.04) requires
merging changes from https://github.com/cpplint/cpplint, which is a
larger undertaking.
@tautschnig
Add empty module_dependencies.txt to make the linter happy
ccce53c 
This check cannot be disabled.
@tautschnig
Enable cpplint regression tests
a5b3008 
We have regression tests for cpplint.py, and should perhaps have used
them to avoid silently failing.
@tautschnig tautschnig removed their assignment Mar 19, 2021
NlightNFotis
NlightNFotis approved these changes Mar 21, 2021
View reviewed changes
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

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

LGTM

@NlightNFotis NlightNFotis merged commit e010007 into diffblue:develop Mar 21, 2021
@tautschnig tautschnig deleted the fix-cpplint-action branch March 22, 2021 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NlightNFotis NlightNFotis NlightNFotis approved these changes

Assignees

@NlightNFotis NlightNFotis

@thomasspriggs thomasspriggs

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tautschnig @NlightNFotis @thomasspriggs