-
-
Notifications
You must be signed in to change notification settings - Fork 758
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 new id for the rules #7313
Add new id for the rules #7313
Conversation
4bb7aff
to
4b73dd7
Compare
4b73dd7
to
2fb58ac
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7313 +/- ##
=========================================
Coverage 84.31% 84.31%
Complexity 4149 4149
=========================================
Files 573 573
Lines 11888 11890 +2
Branches 2458 2458
=========================================
+ Hits 10023 10025 +2
Misses 613 613
Partials 1252 1252 ☔ View full report in Codecov by Sentry. |
2fb58ac
to
cd28075
Compare
0015d49
to
10aed8b
Compare
detekt-api/src/testFixtures/kotlin/io/gitlab/arturbosch/detekt/test/TestFactory.kt
Fixed
Show fixed
Hide fixed
detekt-api/src/testFixtures/kotlin/io/gitlab/arturbosch/detekt/test/TestFactory.kt
Fixed
Show fixed
Hide fixed
10aed8b
to
cd33a1f
Compare
detekt-api/src/testFixtures/kotlin/io/gitlab/arturbosch/detekt/test/TestFactory.kt
Fixed
Show fixed
Hide fixed
detekt-api/src/testFixtures/kotlin/io/gitlab/arturbosch/detekt/test/TestFactory.kt
Fixed
Show fixed
Hide fixed
cd33a1f
to
d638fad
Compare
d638fad
to
0eeaaaf
Compare
0eeaaaf
to
c2f1dfa
Compare
c2f1dfa
to
0225827
Compare
@detekt/maintainers I need reviews here. The PR is big but they are basically renames. The thing that I want you to review is if you agree with the names. Now that we are going to have multiple instances of the same rule we need "two" identifiers. The name of the rule (previously called id) and the id of the instance of that rule. In general we should always use the new "id" but there are use cases for use the name, for example to link to the documentation. Also, this PR renames |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚢
This PR changes some names. The first one is that
Rule
has aName
instead of anId
. A rule can no longer be identified by a static name.The second one is to introduce a new
id
that can be different fromRule.Name
.And because now
RuleInfo
had aname
and anid
I renamed it toRuleInstance
.Naming is REALLY difficult so any help here is more than welcome.
This PR is part of #7263 but because that PR contains a lot of decissions I prefer to split it so we can talk about each one in different PRs but I recomment to read #7263 so you get the full context about why this change is needed.