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 maxErrorsPerFile blocking configuration #986

Merged
merged 5 commits into from
Apr 12, 2023

Conversation

mandrav
Copy link
Contributor

@mandrav mandrav commented Apr 10, 2023

The default max errors per file of 5 is too small IMHO. This commit makes this number user-configurable.

The default max errors per file of 5 is too small IMHO.
This commit makes this number user-configurable.
@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.13 🎉

Comparison is base (74516ca) 93.47% compared to head (64d23c2) 93.61%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #986      +/-   ##
==========================================
+ Coverage   93.47%   93.61%   +0.13%     
==========================================
  Files          63       63              
  Lines        5320     5323       +3     
==========================================
+ Hits         4973     4983      +10     
+ Misses        271      266       -5     
+ Partials       76       74       -2     
Impacted Files Coverage Δ
config/blocking.go 88.63% <100.00%> (+0.54%) ⬆️
lists/list_cache.go 98.53% <100.00%> (+<0.01%) ⬆️
resolver/blocking_resolver.go 97.82% <100.00%> (ø)

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kwitsch
Copy link
Collaborator

kwitsch commented Apr 10, 2023

Looks good to me. 👍

I'll approve it after the linter errors are fixed.

@ThinkChaos ThinkChaos force-pushed the max_errors_per_file branch from 0d84bf6 to b1ffea9 Compare April 12, 2023 16:24
@ThinkChaos
Copy link
Collaborator

Thanks for the PR!

I pushed a couple commits to fix the lint error, add docs and a test, and allow using -1 as the value for "no limit".

FYI we also want to support percentage and switch the default to that, see #966.
I'll try to get that done before the next release so we can that in the config from the start. That'll involve making the config a string, so maybe -1 can be replaced with literally writing "no-limit".

@ThinkChaos ThinkChaos requested a review from kwitsch April 12, 2023 16:52
@kwitsch kwitsch enabled auto-merge (squash) April 12, 2023 18:39
Copy link
Collaborator

@kwitsch kwitsch left a comment

Choose a reason for hiding this comment

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

  • linter ok 👍
  • documentation enhanced 👍
  • unit test coverage 👍

The change to disable the error limit sounds reasonable since it's already an internal constant.

@kwitsch kwitsch merged commit 015b565 into 0xERR0R:main Apr 12, 2023
@mandrav
Copy link
Contributor Author

mandrav commented Apr 12, 2023

Awesome, thanks!

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