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

Disable Style/TrailingCommaInLiteral #31

Merged
merged 1 commit into from
Sep 28, 2020
Merged

Conversation

ghoneycutt
Copy link
Member

When this is enabled, trailing comma's for elements in a hash are not allowed. This is not the style that Puppet uses and causes code churn. This change would disable that functionality.

When this is enabled, trailing comma's for elements in a hash are not allowed. This is not the style that Puppet uses and causes code churn. This change would disable that functionality.
Copy link

@igalic igalic left a comment

Choose a reason for hiding this comment

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

that's an extremely confusing behaviour for a toggle of this name

but i guess the context I'm missing to understand this name and the behaviour, is the specific tastes of the rubocop developers 🤷🏻‍♀️

@ekohl
Copy link
Member

ekohl commented Sep 28, 2020

I actually was thinking about flipping it to consistent_comma, but disabling can be a good first step.

Note there are a few trailing comma style cops. https://docs.rubocop.org/rubocop/0.92/cops.html#department-style lists:

  • Style/TrailingCommaInArguments
  • Style/TrailingCommaInArrayLiteral
  • Style/TrailingCommaInBlockArgs
  • Style/TrailingCommaInHashLiteral

It may have changed a bit in the older version we're using, but I'd prefer to have those all with the same setting.

@alexjfisher
Copy link
Member

Totally agree that it makes no sense to enforce one style in puppet code and the opposite in ruby code.

@ghoneycutt
Copy link
Member Author

@ekohl I reviewed those cops you posted and they actually make sense as they are and are in line with puppet styling.

@ghoneycutt ghoneycutt merged commit fce89a5 into master Sep 28, 2020
@ghoneycutt ghoneycutt deleted the ghoneycutt-patch-1 branch September 28, 2020 17:26
@ekohl
Copy link
Member

ekohl commented Sep 29, 2020

@ghoneycutt I think it's improper to merge your own PRs if someone had a comment about it that received multiple 👍. Especially if it's just within a few hours. Either you should point out why it doesn't need to be addressed or fix it. IMHO Voxpupupli is based on collaboration and consensus. That also means giving time for a discussion.

@ghoneycutt
Copy link
Member Author

@ekohl I read Alex's post and the corresponding +1's are in agreement with the change.

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.

4 participants