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

Discourage new Case files #2399

Merged
merged 2 commits into from
Mar 4, 2020
Merged

Discourage new Case files #2399

merged 2 commits into from
Mar 4, 2020

Conversation

3flex
Copy link
Member

@3flex 3flex commented Mar 4, 2020

Closes #1089

That issue will remain open until all case files are retired, which may take a long time as it's quite a thankless task - instead, add a comment asking contributors not to add new case files with a reference to the PR so the PR can be closed.

@codecov-io
Copy link

codecov-io commented Mar 4, 2020

Codecov Report

Merging #2399 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2399   +/-   ##
=========================================
  Coverage     82.87%   82.87%           
  Complexity     2148     2148           
=========================================
  Files           353      353           
  Lines          6119     6119           
  Branches       1118     1118           
=========================================
  Hits           5071     5071           
  Misses          475      475           
  Partials        573      573

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 9a863b7...4f1a1d3. Read the comment docs.

@@ -4,6 +4,8 @@ import io.gitlab.arturbosch.detekt.test.resource
import java.nio.file.Path
import java.nio.file.Paths

/* Do not add new elements to this file.
Copy link
Member

Choose a reason for hiding this comment

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

Lets encourage the usage of inline snippets here?

Copy link
Member

Choose a reason for hiding this comment

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

I agree with Artur.
Furthermore, we should also strive to inline the case files when we change something in a rule or in the corresponding spec file, which still links a case file.

Copy link
Member

Choose a reason for hiding this comment

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

I also try to mention this in PR reviews.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've reworded, hopefully the updated text is OK?

@schalkms schalkms merged commit e390c0c into master Mar 4, 2020
@schalkms schalkms deleted the 3flex-patch-1 branch March 4, 2020 17:19
@arturbosch arturbosch added this to the 1.7.0 milestone Mar 5, 2020
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.

Cases files are super rigorous to work with
4 participants