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

Create detekt-rules-ruleauthors module #5129

Merged
merged 1 commit into from Sep 6, 2022
Merged

Conversation

BraisGabin
Copy link
Collaborator

This is a new first-party detekt plugin for rules that only rule authors will need. In the next PR I'll add the first rule to this rule-set.

Any idea with the naming is more than welcome. I don't like detekt-authors too much but detekt-detekt seems even worst.

@github-actions github-actions bot added the build label Jul 24, 2022
@BraisGabin BraisGabin added this to the 1.22.0 milestone Jul 24, 2022
@codecov
Copy link

codecov bot commented Jul 24, 2022

Codecov Report

Merging #5129 (f6692e1) into main (6b6d904) will not change coverage.
The diff coverage is n/a.

Current head f6692e1 differs from pull request most recent head 6d37691. Consider uploading reports for the commit 6d37691 to get more accurate results

@@     Coverage Diff      @@
##   main   #5129   +/-   ##
============================
============================

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


override val ruleSetId: String = "detekt"

override fun instance(config: Config) = RuleSet(ruleSetId, @Suppress("UseEmptyCounterpart") listOf())
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:detekt-generator relies in the fact that there is a listOf() in this file. If I use emptyList() it fails. This is an edge-transitory-case so I'm just Suppressing it. In the next PR I'll have a rule here so I'll remove @Suppress

@cortinico
Copy link
Member

I don't like detekt-authors too much but detekt-detekt seems even worst.

I'm also not a big fan of the name. I would go for either

  • detekt-rules-customrules
  • detekt-rules-authors
  • detekt-rules-ruleauthors

@BraisGabin
Copy link
Collaborator Author

BraisGabin commented Aug 1, 2022

I didn't want to start it by detekt-rules because detekt-rules are the detekt's standard rules. I imagine that's the reason for detekt-formatting to be called like that instead of detekt-rules-formatting. But detekt-ruleauthors feels like a better name.

Other option is to move these to another repo under the detekt organization. That could also have sense. We talked about moving detekt-formatting out of these repo so maybe is a good thing to directly create these rules out of the main repo.

If we move it to another repo the name is not an issue anymore. We can call it detekt-rules directly.

@cortinico
Copy link
Member

I imagine that's the reason for detekt-formatting to be called like that instead of detekt-rules-formatting

Since we are at this: IMHO detekt-formatting is not the best name. I would have called it detekt-rules-formatting OR detekt-ktlint-wrapper.

I think that having the assumtion that detekt-rules-* contains custom rules is sound, and would make the codebase easier to discover.

Other option is to move these to another repo under the detekt organization. That could also have sense. We talked about moving detekt-formatting out of these repo so maybe is a good thing to directly create these rules out of the main repo.

That's also doable, but it really depends on how often we expect those rules to change. Maybe they can start off inside the Detekt repo and being moved outside in the future.

@BraisGabin
Copy link
Collaborator Author

I imagine that's the reason for detekt-formatting to be called like that instead of detekt-rules-formatting

Since we are at this: IMHO detekt-formatting is not the best name. I would have called it detekt-rules-formatting OR detekt-ktlint-wrapper.

Agree, detekt-ktlint-wrapper seems a better name. Or just detekt-ktlint. Because there are other formatting tools.

I think that having the assumtion that detekt-rules-* contains custom rules is sound, and would make the codebase easier to discover.

That have sense, true.


So I'm renaming it to detekt-rules-ruleauthors for the gradle subproject.

@github-actions
Copy link

github-actions bot commented Aug 7, 2022

Messages
📖 Thanks for adding a new rule to Detekt ❤️

Generated by 🚫 dangerJS against 6d37691

@BraisGabin BraisGabin merged commit 5d179e1 into main Sep 6, 2022
21 checks passed
@BraisGabin BraisGabin deleted the create-authors-rules branch September 6, 2022 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build notable changes Marker for notable changes in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants