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

rubocop: Use Sorbet/StrictSigil as it's better than comments #18023

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

issyl0
Copy link
Member

@issyl0 issyl0 commented Aug 12, 2024

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

  • Part of Use Sorbet typed: strict in all (non-package) files in Homebrew organisation #17297.
  • Previously I thought that comments were fine to discourage people from wasting their time trying to bump things that used undef that Sorbet didn't support. But RuboCop is better at this since it'll complain if the comments are unnecessary.
  • Suggested in ci: Encourage Sorbet # typed: strict in new code #18018 (comment).
  • I've gone for a mixture of rubocop:disable for the files that can't be typed: strict (use of undef, required before everything else, etc) and rubocop:todo for everything else that should be tried to make strictly typed. There's no functional difference between the two as rubocop:todo is rubocop:disable with a different name.
  • And I entirely disabled the cop for the docs/ directory since typed: strict isn't going to gain us anything for some Markdown linting config files.

  • This means that now it's easier to track what needs to be done rather than relying on checklists of files in our big Sorbet issue:
$ git grep 'typed: true # rubocop:todo Sorbet/StrictSigil' | wc -l
    268
  • And this is confirmed working for new files:
$ git status
On branch use-rubocop-for-sorbet-strict-sigils
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        Library/Homebrew/bad.rb
        Library/Homebrew/good.rb

nothing added to commit but untracked files present (use "git add" to track)

$ brew style
Offenses:

bad.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got true.
# typed: true
^^^^^^^^^^^^^

1340 files inspected, 1 offense detected

@issyl0 issyl0 force-pushed the use-rubocop-for-sorbet-strict-sigils branch from 8fc4738 to 321d15d Compare August 12, 2024 10:16
@issyl0 issyl0 requested a review from MikeMcQuaid August 12, 2024 10:17
@MikeMcQuaid
Copy link
Member

Thanks @issyl0! Unfortunately there's now a bunch of conflicts but good to self-merge once those are resolved.

- Previously I thought that comments were fine to discourage people from
  wasting their time trying to bump things that used `undef` that Sorbet
  didn't support. But RuboCop is better at this since it'll complain if
  the comments are unnecessary.

- Suggested in #18018 (comment).

- I've gone for a mixture of `rubocop:disable` for the files that can't
  be `typed: strict` (use of undef, required before everything else, etc)
  and `rubocop:todo` for everything else that should be tried to make
  strictly typed. There's no functional difference between the two as
  `rubocop:todo` is `rubocop:disable` with a different name.

- And I entirely disabled the cop for the docs/ directory since
  `typed: strict` isn't going to gain us anything for some Markdown
  linting config files.

- This means that now it's easier to track what needs to be done rather
  than relying on checklists of files in our big Sorbet issue:

```shell
$ git grep 'typed: true # rubocop:todo Sorbet/StrictSigil' | wc -l
    268
```

- And this is confirmed working for new files:

```shell
$ git status
On branch use-rubocop-for-sorbet-strict-sigils
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        Library/Homebrew/bad.rb
        Library/Homebrew/good.rb

nothing added to commit but untracked files present (use "git add" to track)

$ brew style
Offenses:

bad.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got true.
^^^^^^^^^^^^^

1340 files inspected, 1 offense detected
```
@issyl0 issyl0 force-pushed the use-rubocop-for-sorbet-strict-sigils branch from 321d15d to 4597843 Compare August 12, 2024 14:26
@issyl0 issyl0 enabled auto-merge August 12, 2024 14:31
@issyl0 issyl0 merged commit 1376c5d into master Aug 12, 2024
32 checks passed
@issyl0 issyl0 deleted the use-rubocop-for-sorbet-strict-sigils branch August 12, 2024 14:39
@Paugho

This comment was marked as spam.

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