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

config/default: Unset DisabledByDefault: true #119

Merged
merged 18 commits into from
Oct 13, 2022

Commits on Oct 10, 2022

  1. config/default: Unset DisabledByDefault: true

    - The `DisabledByDefault` config option made it so that "all cops in the
      default configuration are disabled, and only cops in user configuration
      are enabled", which makes cops "opt-in rather than opt-out"
      (source: https://github.com/rubocop/rubocop/blob/d1270c468cdb9a211229e36aef8d568ae3bfe607/config/default.yml#L91-L102).
    
    - It's not immediately obvious that this gem has this config option,
      and it's used in a lot of GitHub Ruby projects. This means that
      people may write new, custom cops in downstream projects, with no
      configuration in `.rubocop.yml`, and then they never run because all
      cops are disabled unless explicitly enabled or otherwise configured
      with `Include`/`Exclude`.
    
    - This change is a follow-up to the great conversations had in
      #117, and makes this gem
      take a more neutral stance to match whatever RuboCop's defaults are
      for enabling (or not) cops. Then it's up to the maintainers of this
      gem and the deciders of our styleguide to pick the settings for the
      rules to include, and/or it's up to the downstream consumers of this
      gem to decide whether they want all the rules or just some of the rules
      as they are defined in here.
    issyl0 authored Oct 10, 2022
    Configuration menu
    Copy the full SHA
    1b549a3 View commit details
    Browse the repository at this point in the history
  2. rubocop.yml: Disable some cops locally that aren't in the styleguide

    - This config applies only to this repo, it's not the global config.
    issyl0 authored Oct 10, 2022
    Configuration menu
    Copy the full SHA
    7df97a7 View commit details
    Browse the repository at this point in the history
  3. Run bundle exec rubocop -a (or -A) on all the code in this repo

    - 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
    issyl0 authored Oct 10, 2022
    Configuration menu
    Copy the full SHA
    53a0a37 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    e36d683 View commit details
    Browse the repository at this point in the history

Commits on Oct 11, 2022

  1. README: Note that DisabledByDefault: true is useful for granularity

    - 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).
    issyl0 authored Oct 11, 2022
    Configuration menu
    Copy the full SHA
    ee3cac8 View commit details
    Browse the repository at this point in the history
  2. CHANGELOG: Add an 'unreleased' section

    - Should probably write about the recent accessibility gem changes here, too, but that's not in this PR. Hence a generic 'unreleased' section.
    issyl0 authored Oct 11, 2022
    Configuration menu
    Copy the full SHA
    db3b338 View commit details
    Browse the repository at this point in the history
  3. config/default: Explicitly set Enabled: false for a bunch of rules

    - 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.
    issyl0 authored Oct 11, 2022
    Configuration menu
    Copy the full SHA
    f40758f View commit details
    Browse the repository at this point in the history
  4. config/default: Add Enabled: true cops now configured by default

    - 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" }
    ```
    issyl0 authored Oct 11, 2022
    Configuration menu
    Copy the full SHA
    05e8276 View commit details
    Browse the repository at this point in the history

Commits on Oct 12, 2022

  1. config/default: Set Enabled: false on a bunch of rules

    - 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.
    issyl0 authored Oct 12, 2022
    Configuration menu
    Copy the full SHA
    e48f452 View commit details
    Browse the repository at this point in the history
  2. config/default: Fix Layout/IndentationStyle configuration

    - The alphabetization script didn't cope very well with all the extra options for some reason.
    issyl0 authored Oct 12, 2022
    Configuration menu
    Copy the full SHA
    baa90b2 View commit details
    Browse the repository at this point in the history
  3. .rubocop.yml: Move Enabled: false cops into config/default.yml

    - 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.
    issyl0 authored Oct 12, 2022
    Configuration menu
    Copy the full SHA
    5a1ede7 View commit details
    Browse the repository at this point in the history
  4. config/default: It's impossible to disable Lint/Syntax

    - 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.
    ```
    issyl0 authored Oct 12, 2022
    Configuration menu
    Copy the full SHA
    60a3a04 View commit details
    Browse the repository at this point in the history
  5. Revert "Run bundle exec rubocop -a (or -A) on all the code in thi…

    …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.
    issyl0 authored Oct 12, 2022
    Configuration menu
    Copy the full SHA
    55b4ece View commit details
    Browse the repository at this point in the history
  6. Revert "Fix Naming/MemoizedInstanceVariableName offenses"

    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.
    issyl0 authored Oct 12, 2022
    Configuration menu
    Copy the full SHA
    d95509e View commit details
    Browse the repository at this point in the history
  7. config/default: Set Enabled: false for new RuboCop rules

    - 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
    ```
    issyl0 authored Oct 12, 2022
    Configuration menu
    Copy the full SHA
    70dc91b View commit details
    Browse the repository at this point in the history

Commits on Oct 13, 2022

  1. Configuration menu
    Copy the full SHA
    4274086 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    cdcad37 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    6bbad25 View commit details
    Browse the repository at this point in the history