Skip to content
This repository was archived by the owner on Jan 22, 2026. It is now read-only.

[pull] master from github:master#4

Open
pull[bot] wants to merge 176 commits intoMattlk13:masterfrom
github:master
Open

[pull] master from github:master#4
pull[bot] wants to merge 176 commits intoMattlk13:masterfrom
github:master

Conversation

@pull
Copy link

@pull pull bot commented Aug 29, 2019

See Commits and Changes for more details.


Created by pull[bot]. Want to support this open source service? Please star it : )


This change is Reviewable

jasonkarns and others added 3 commits August 1, 2019 17:17
This change is necessary because access modifiers within `concerning` blocks are not useless.

[ActiveSupport::Module::Concerning](https://api.rubyonrails.org/classes/Module/Concerning.html)'s block creates a context in which access modifiers are respected.

see rubocop/rubocop#3422
https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Lint/UselessAccessModifier
add support for render ClassName
@pull pull bot added the ⤵️ pull label Aug 29, 2019
joelhawksley and others added 26 commits August 30, 2019 10:34
ActiveSupport concerning supports access modifiers
Update for rubocop ≥ 0.7x
Ruby 2.3.x has reached end of life, and is no longer supported.
Ruby 2.7.x has been released.
Update Rubies configured for Travis CI
The gem implicitly assumes that these are included, but due to version dependency
hell, we were unable to include them earlier.
This PR has pinned Rubocop to v.0.75.0, and the cops were renamed in later
versions of Rubocop. I'll rename the cops again as we incrementally bump
the rubocop version.
This was a breaking change, and I had to rename a couple of cops
to get the build passing.
This is a breaking change.

I've updated cops that got renamed.
This is a breaking change.

To get this to pass CI, I have done the following:

* deleted performance cops
* renamed a Metrics/LineLength to Layout/LineLength
* deleted another Metrics/LineLength

The performance cops appear to have been extracted into a separate gem,
https://github.com/rubocop-hq/rubocop-performance and as of 0.78.0
referencing them without including the rubocop-performance gem as a
dependency blows up.
This is a breaking change. To get this to pass CI
I had to delete the 'Lint/EndInMethod' cop, which has
been renamed to 'Style/EndBlock'. Since we already had
Style/EndBlock, I deleted rather than renaming the old
cop.
This is a breaking change.

In order to pass CI, I had to rename the Layout/Tab cop
to Layout/IndentationStyle.
Add performance and rails rubocop gems explicitly
issyl0 and others added 28 commits October 10, 2022 19:43
- This config applies only to this repo, it's not the global config.
- These rules seemed to make sense and were all autocorrectable:
  - Style/SymbolArray
  - Layout/EmptyLineAfterGuardClause
  - Style/MultipleComparison
  - Style/IfUnlessModifier (since we don't have line length limitations)
  - Style/EmptyCaseCondition
  - Style/TrailingUnderscoreVariable
  - Style/RedundantParentheses
  - Lint/UnusedBlockArgument
  - Style/NegatedIf
  - Style/StringConcatenation
  - Style/SafeNavigation
  - Layout/MultilineMethodCallIndentation
  - Layout/EmptyLineAfterMagicComment
  - Layout/SpaceInsideHashLiteralBraces
  - Layout/EmptyLines
  - Layout/EmptyLineBetweenDefs
…-code-block

fix(STYLEGUIDE.md): remove extra opening code block
- Now that this behaviour has changed in `rubocop-github`, our users
  might want to have finer-grained control of which cops they enable
  since they can't rely on this gem to include that option now (if they
  even noticed).
- Should probably write about the recent accessibility gem changes here, too, but that's not in this PR. Hence a generic 'unreleased' section.
- These rules come directly from RuboCop. They're configured `Enabled: false`
  upstream, but it's worth being explicit in this gem since this guards
  against RuboCop changing underneath us and causing linting changes that
  either don't match our styleguide or we don't want to propagate to
  users of this gem.
- This was generated by the following script (thanks to `@matthewd`),
  where `show-cops.yml` is the output of
  `bundle exec rubocop --show-cops`:

```ruby
require "yaml"
y = YAML.unsafe_load_file("show-cops.yml")
cfg = YAML.unsafe_load_file(".rubocop.yml")
off = y.keys.select { |k| y[k]["Enabled"] == false }
(off - cfg.keys).sort.each { |k| puts "#{k}:\n  Enabled: false\n\n" }
```

- Next I'll do the same for `Enabled: true`, then in a follow-up commit
  or PR (probably PR, at this point) I'll go through and link the
  `Enabled: true` ones to their styleguide items in the GitHub styleguide.
- Previously these were disabled because of `DisableByDefault: true`.
  Since we removed that config, all of these become enabled. That's
  somewhat undesirable since when we ship the new version, downstream
  consumers of this gem will have to make decisions to enable or disable
  the cops. We should do this for them and make this gem the source of
  truth matching the styleguide as far as possible. Of course, downstream
  projects are free to tweak in their own `.rubocop.yml` for what matches
  their project style, but we should attempt to set a central standard
  according to the styleguide that we publish, to make writing Ruby at
  GitHub easier.

- I left some TODOs to audit these (before we ship the new gem version!)
  for whether or not they:
  - a) already exist in the file in either enabled/disabled state;
  - b) checking if the GitHub Ruby styleguide has opinions on them and enabling/disabling accordingly.

- This list was (as in the previous commit) generated by:

```ruby
require "yaml"
y = YAML.unsafe_load_file("show-cops.yml")
cfg = YAML.unsafe_load_file(".rubocop.yml")
on = y.keys.select { |k| y[k]["Enabled"] == true }
(on - cfg.keys).sort.each { |k| puts "#{k}:\n  Enabled: true\n\n" }
```
- This enumerates all of the currently defined rules that were
  implicitly disabled by `DisabledByDefault: true` and make them
  explicitly disabled so that there is no functional change to the
  RuboCop cops today e.g. if someone upgrades they won't experience a
  flood of newly enabled rules.
- This was achieved by briefly re-enabling `DisabledByDefault`, then
  running `bundle exec rubocop --show-cops`, and the script included
  with the previous commit.
- All of the TODOs from the previous commit about cross-checking newly
  enabled rules with the styleguide are gone, since there are no newly
  enabled rules, and this should be a better user experience for
  downstream consumers since they don't have to scramble to understand
  new RuboCop rules that fail in their project (over and above custom
  ones they have in their project which will now be enabled because we
  removed `DisabledByDefault` in this gem).
- In the future, we'll go through the list of rules here and cross-check
  them with the styleguide, potentially migrating some of them to
  `Enabled: true`. But this changeset as it is now should be good to ship.
- The alphabetization script didn't cope very well with all the extra options for some reason.
- We've now changed tactic such that we're not enabling any new cops for
  downstream consumers, so let's move the "locally" disabled cops for
  this codebase back into `config/default.yml` so that we don't enable
  them for downstream consumers by accident. Something like
  `Lint/AssignmentInCondition` that doesn't have an autocorrect would be
  particularly annoying to receive violations for when you're just
  trying to upgrade your project's version of this gem.
- Somehow this snuck in, but RuboCop errors:

```
Error: configuration for Syntax cop found in config/default.yml.
It's not possible to disable this cop.
```
…s repo"

This reverts commit 53a0a37.

This is because we've now disabled all the rules that would have been
`DisabledByDefault`. This is so that downstream consumers don't get any
nasty surprises. As a result, there should be no RuboCop linting
violations in the code in this repo, so we can back out the changes to
slim the diff: they're now redundant.

Of course, it would be a good idea to go and make these changes at a later
date, but we can do that when we enable the correct rules having matched
each one up with its rationale in our styleguide.
This reverts commit e36d683.

 This is because we've now disabled all the rules that would have been
`DisabledByDefault`. This is so that downstream consumers don't get any
nasty surprises. As a result, there should be no RuboCop linting
violations in the code in this repo, so we can back out the changes to
slim the diff: they're now redundant.

Of course, it would be a good idea to go and make these changes at a later
date, but we can do that when we enable the correct rules having matched
each one up with its rationale in our styleguide.
- The following message appeared when I ran `bundle exec rubocop` on this
  project. Since we're setting everything to `Enabled: false` to mimic
  `DisabledByDefault` for now, let's make this message go away. We almost
  certainly also don't want to blanket enable all new cops, so I've not
  added that configuration.

```
The following cops were added to RuboCop, but are not configured. Please set Enabled to either `true` or `false` in your `.rubocop.yml` file.

Please also note that you can opt-in to new cops by default by adding this to your config:
  AllCops:
    NewCops: enable

Lint/EmptyBlock: # new in 1.1
  Enabled: true
Lint/EmptyClass: # new in 1.3
  Enabled: true
Lint/EmptyInPattern: # new in 1.16
  Enabled: true
For more information: https://docs.rubocop.org/rubocop/versioning.html
```
…cops and rules to disambiguate rule inheritance from custom cops
Separate Ruby (`rubocop-github`) from Rails (`rubocop-github-rails`) cops and rules to disambiguate rule inheritance from custom cops
config/default: Unset `DisabledByDefault: true`
fix(STYLEGUIDE.md): use list item instead of codeblock
@bensheldon bensheldon deleted the branch Mattlk13:master October 13, 2022 15:26
@bensheldon bensheldon deleted the master branch October 13, 2022 15:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.