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

Equals() smells #297

Closed
wants to merge 5 commits into from
Closed

Equals() smells #297

wants to merge 5 commits into from

Conversation

schalkms
Copy link
Member

No description provided.


class EqualsNullCall(config: Config = Config.empty) : Rule(config) {

override val issue = Issue("EqualsWithHashCodeExist", Severity.Defect,
Copy link
Member

Choose a reason for hiding this comment

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

Forgot to rename the issue id

Copy link
Member

Choose a reason for hiding this comment

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

Why is this a defect? This looks like a style check for kotlin newcomers ?


class WrongEqualsTypeParameter(config: Config = Config.empty) : Rule(config) {

override val issue = Issue("WrongEqualsTypeParameter", Severity.Defect,
Copy link
Member

Choose a reason for hiding this comment

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

Like this one!

@@ -38,8 +38,14 @@ potential-bugs:
active: true
DuplicateCaseInWhenExpression:
active: true
EqualsAlwaysReturnsTrueOrFalse:
active: true
Copy link
Member

Choose a reason for hiding this comment

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

Please turn all the rules to active: false. Will turn them on after some versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to be careful with this though I'd expect them to only get turned on by default with a major bump like 1.x to 2.x

Another solution would be to keep having detekt in RC for a few more weeks to gather some more feedback

Copy link
Member

Choose a reason for hiding this comment

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

I think the WrongEqualsTypeParameter is rule is like a must have for 1.0. AlwaysTrueOrFalse is good too. Why would you expect a major version bump?

Yeah I want to have a good 1.0 release and I think I won't turn on any new rules active until then.
I will also need some weeks for the wiki to be finished ... so little time :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly because the other code quality tools do it like this. But obviously we could do this differently however we should point this out in the README then.

@arturbosch
Copy link
Member

Manually merged via 54f3787

@arturbosch arturbosch closed this Aug 14, 2017
@arturbosch arturbosch modified the milestone: next Aug 17, 2017
@schalkms schalkms deleted the equals-smells branch September 20, 2017 16:43
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.

None yet

3 participants