Skip to content

Correct maxIssues documentation#3456

Merged
BraisGabin merged 1 commit into
detekt:masterfrom
artnc:patch-1
Feb 8, 2021
Merged

Correct maxIssues documentation#3456
BraisGabin merged 1 commit into
detekt:masterfrom
artnc:patch-1

Conversation

@artnc

@artnc artnc commented Feb 8, 2021

Copy link
Copy Markdown
Contributor

This repo's own config contains maxIssues: 0, which only makes sense under this PR's new wording since it'd be impossible for the build to ever pass with maxIssues: 0 if the current wording were true.

The current wording led to an issue in Duolingo's codebase where we were declaring maxIssues: 1 to mean "break if at least one issue is found" and unknowingly allowing our repo to always contain exactly 1 unfixed violation. This confused contributors because two new PRs each introducing one violation would often be merged around the same time into master, adding up to two violations and breaking master.

@codecov

codecov Bot commented Feb 8, 2021

Copy link
Copy Markdown

Codecov Report

Merging #3456 (d212474) into master (b120ffb) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3456   +/-   ##
=========================================
  Coverage     80.26%   80.26%           
  Complexity     2785     2785           
=========================================
  Files           454      454           
  Lines          8418     8418           
  Branches       1609     1609           
=========================================
  Hits           6757     6757           
  Misses          789      789           
  Partials        872      872           

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 b120ffb...df47b10. Read the comment docs.

@picklebento picklebento left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lately, we updated the CLI args https://detekt.github.io/detekt/cli.html and I verified the language there is accurate. Thank you for improving the accuracy of our documentation!

@BraisGabin BraisGabin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for the fix!

@BraisGabin BraisGabin merged commit e8d7e9e into detekt:master Feb 8, 2021
@cortinico cortinico added the housekeeping Marker for housekeeping tasks and refactorings label Feb 19, 2021
@cortinico cortinico added this to the 1.16.0 milestone Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

housekeeping Marker for housekeeping tasks and refactorings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants