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

Consider deprecated rules as inactive when running allRules #6381

Merged

Conversation

marschwar
Copy link
Contributor

@marschwar marschwar commented Aug 12, 2023

(quick) fixes #6336

Note: This PR targets the release/1.x branch instead of main.

Following this suggestion I hacked something up that should fix the issue short term. The idea is that all deprecated rules are considered as inactive in case detekt is run with the allRules option. We should probably find a better solution for the main branch.

@codecov
Copy link

codecov bot commented Aug 12, 2023

Codecov Report

❗ No coverage uploaded for pull request base (release/1.x@5fc3df0). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             release/1.x    #6381   +/-   ##
==============================================
  Coverage               ?   84.93%           
  Complexity             ?     4043           
==============================================
  Files                  ?      569           
  Lines                  ?    13545           
  Branches               ?     2393           
==============================================
  Hits                   ?    11504           
  Misses                 ?      873           
  Partials               ?     1168           

@BraisGabin
Copy link
Member

🤔 I'm not sure here. But what will happen if you read the deprecation file? You should be able to extract that list too, right? This way you don't need to hardcode the list of deprecated rules.

@marschwar
Copy link
Contributor Author

But what will happen if you read the deprecation file? You should be able to extract that list too, right? This way you don't need to hardcode the list of deprecated rules.

I totally agree. This could be the actual fix for the main branch. This was meant to be a quick fix only in case we want to deliver something quickly. But I guess that still could be done here. I will update the PR.

@BraisGabin
Copy link
Member

Then this PR should go against 2.0, right?

@marschwar
Copy link
Contributor Author

Then this PR should go against 2.0, right?

No, it is intended for the 1.x only. For the 2.x I would like to use #6382.

@marschwar marschwar added this to the 1.23.2 milestone Aug 14, 2023
Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

Works great for me 👍 Let's merge this so I can prepare .2

@marschwar
Copy link
Contributor Author

marschwar commented Aug 19, 2023

Fix for 2.0 is #6413

@BraisGabin
Copy link
Member

We can merge this, right?

@cortinico
Copy link
Member

We can merge this, right?

Yup 👍

@cortinico cortinico merged commit c0684d1 into detekt:release/1.x Sep 1, 2023
23 checks passed
@marschwar marschwar deleted the fix/6336_deactivateDeprecatedRules branch September 1, 2023 21:23
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