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

Change variable min & max length to match IntelliJ. #635

Merged
merged 3 commits into from Jan 1, 2018
Merged

Change variable min & max length to match IntelliJ. #635

merged 3 commits into from Jan 1, 2018

Conversation

vanniktech
Copy link
Contributor

If you think it's better that way I can adjust the tests.

Copy link
Collaborator

@Mauin Mauin left a comment

Choose a reason for hiding this comment

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

Config & Documentation will still need an update

Copy link
Collaborator

@Mauin Mauin left a comment

Choose a reason for hiding this comment

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

@author tags in the rule files maybe. 👍

@arturbosch
Copy link
Member

arturbosch commented Dec 21, 2017

Hmm it kinda looks to me as if the kotlin devs were to lazy to check to ignore variable names in loops (@ min length 1) :/

@vanniktech
Copy link
Contributor Author

I don't quite get what you mean with your last comment artur.

@arturbosch
Copy link
Member

arturbosch commented Dec 24, 2017

In loops names like i (for i in ints {}) is ok. All other variables should not be called a, b, c ... i or whatself. It feels kinda hacky to set this defaults ourself :(

I'm feeling like we should be more opiniated in this case :/, what do you think?

@vanniktech
Copy link
Contributor Author

I do think having a min value of 1 is fine. I wouldn't go down the street of separating them via loops properties.

@arturbosch arturbosch merged commit 8031120 into detekt:master Jan 1, 2018
@arturbosch arturbosch added this to the RC6-1 milestone Jan 1, 2018
@vanniktech vanniktech deleted the variabledefaults branch January 1, 2018 22:19
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