Skip to content

config/default: Link some rules to their styleguide justifications - #124

Merged
issyl0 merged 1 commit into
mainfrom
match-styleguide-to-rubocop
Oct 19, 2022
Merged

config/default: Link some rules to their styleguide justifications#124
issyl0 merged 1 commit into
mainfrom
match-styleguide-to-rubocop

Conversation

@issyl0

Copy link
Copy Markdown
Member
  • "Some of", yes, since there are a lot. But I thought I'd get started since this is an important step towards being able to a) enable more RuboCop rules based on the evidence of our styleguide, b) realising how far adrift our RuboCop configs have potentially become from what's written in the Styleguide, c) encouraging honest adoption of these rules across GitHub because they're actually justified in text.

- "Some of", yes, since there are a lot. But I thought I'd get started
since this is an important step towards being able to a) enable more
RuboCop rules based on the evidence of our styleguide, b) realising
how far adrift our RuboCop configs have potentially become from what's
written in the Styleguide, c) encouraging honest adoption of these
rules across GitHub because they're actually justified in text.
@issyl0
issyl0force-pushed the match-styleguide-to-rubocop branch from 784120d to 596f143CompareOctober 19, 2022 10:59
@issyl0
issyl0 merged commit 104d88e into mainOct 19, 2022
@issyl0
issyl0 deleted the match-styleguide-to-rubocop branch October 19, 2022 11:01

@sampartsampart left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks so much for doing this. One retrospective comment.

Comment threadconfig/default.yml
# TODO: Enable this since it's in the styleguide.
Lint/UnusedBlockArgument:
Enabled: false
StyleGuide: https://github.com/github/rubocop-github/blob/main/STYLEGUIDE.md#spaces-around-equals

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Oops, yes it should! Feel free to submit a PR!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@issyl0@elenatanasoiu@sampart