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

lint: Report all lint errors instead of early exit #28862

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Nov 13, 2023

all-lint.py currently collects all failures. However, the 06_script.sh does not, since July this year (#28103 (comment)).

Fix this by printing all failures before exiting.

Can be tested by modifying (for example) two subtrees in the same commit and then running the linters.

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 13, 2023

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK TheCharlatan, kevkevinpal

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

@DrahtBot DrahtBot added the Tests label Nov 13, 2023
Copy link
Contributor

@TheCharlatan TheCharlatan left a comment

Choose a reason for hiding this comment

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

lgtm ACK fa01f88

I checked that the CI can now correctly report multiple lint failures.

@kevkevinpal
Copy link
Contributor

ACK fa01f88

tested on my own fork and saw multiple lint errors
https://github.com/kevkevinpal/bitcoin/pull/9/checks?check_run_id=18865861937

@maflcko
Copy link
Member Author

maflcko commented Nov 21, 2023

tested on my own fork and saw multiple lint errors

Yeah, I guess linters should enforce their rules globally, instead of only on the diff. Otherwise, one gets different results, depending on how the diff is calculated.

Though, this is unrelated to this pull request and can be improved in the future, if there is need and interest.

@fanquake fanquake merged commit 172cd92 into bitcoin:master Nov 22, 2023
16 checks passed
@maflcko maflcko deleted the 2311-lint-all-err- branch November 23, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants