Skip to content

rubocop: add shim and command.#10239

Merged
MikeMcQuaid merged 3 commits intoHomebrew:masterfrom
MikeMcQuaid:rubocop-shim
Jan 8, 2021
Merged

rubocop: add shim and command.#10239
MikeMcQuaid merged 3 commits intoHomebrew:masterfrom
MikeMcQuaid:rubocop-shim

Conversation

@MikeMcQuaid
Copy link
Copy Markdown
Member

Add a shim and a command that can be used to easily add a single directory to your PATH (Library/Homebrew/shims/gems) and have it automatically install, configure and run rubocop so you can use it for in-editor integrations.

CC @Rylan12 who talked about this with me. ideally we'd/you'd add a commit to this PR with the Visual Studio Code configuration to run this rubocop

@MikeMcQuaid MikeMcQuaid requested a review from Rylan12 January 6, 2021 15:23
@BrewTestBot
Copy link
Copy Markdown
Contributor

Review period will end on 2021-01-07 at 15:23:01 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Jan 6, 2021
@Rylan12
Copy link
Copy Markdown
Member

Rylan12 commented Jan 6, 2021

Just glancing at the changes (haven't tried anything locally yet) makes sense to me.

My limited vscode experience tells me that we can provide a .vscode directory in this repo with settings like these, but this (I believe) will only be effective if we open the root directory in vscode. What I mean is opening a single file (e.g. code $(brew --repo)/Library/Homebrew/cmd/uses.rb) won't inherit these settings.

Another option might be to set up a specific workspace file. Then, maybe provide a brew code command that will open the workspace? Just not totally sure what the right approach to providing a configuration is.

And we also want to give users the ability to modify the configuration for themselves without creating unstaged changes or accidentally committing configuration things in a PR.

@RandomDSdevel
Copy link
Copy Markdown
Contributor

     Why wouldn't this be a developer command, @MikeMcQuaid?

@Rylan12
Copy link
Copy Markdown
Member

Rylan12 commented Jan 7, 2021

Upon running brew rubocop, I get the following output. Not sure if this is an issue on my end or not:

brew rubocop --version
warning: parser/current is loading parser/ruby26, which recognizes
warning: 2.6.6-compliant syntax, but you are running 2.6.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
1.7.0

This warning also shows up in vscode unless ruby.rubocop.suppressRubocopWarnings is true.

To test, I ran gem uninstall rubocop and then ran brew rubocop on both files in this repo and some formula files from homebrew/core. I intentionally created a few style issues to check that everything works properly, and I found no issues.

I did notice that /usr/local/Homebrew/Library/Homebrew/shims/gems/rubocop is consistently slower than just rubocop. Probably because it's invoking brew. Looking at the output of time, the shim is around 0.2 seconds slower, but it was noticeable to me (especially in vscode).


One thing to note: both of these require adding some extensions to vscode. Not sure if we can provide a settings file that includes those or not.

I've also added a vscode settings file that works for me. Unfortunately, I couldn't figure out how to infer HOMEBREW_REPOSITORY so I hardcoded it as /usr/local/Library/Homebrew/shims/gems/. I tried using $(brew --repository)/Library/Homebrew/shims/gems/ with no luck.

I also found that if PATH is set to include Library/Homebrew/shims/gems and vscode is launched using the code CLI tool, the default settings will work correctly.

A note on this approach: this will likely cause issues if users have their own .vscode directory where they've saved their existing settings since the .vscode directory was ignored.

As I said, I'm not really sure what the best way to do things here is, so feel free to just revert/drop if what I'm pushing isn't helpful at all.

@MikeMcQuaid
Copy link
Copy Markdown
Member Author

My limited vscode experience tells me that we can provide a .vscode directory in this repo with settings like these, but this (I believe) will only be effective if we open the root directory in vscode. What I mean is opening a single file (e.g. code $(brew --repo)/Library/Homebrew/cmd/uses.rb) won't inherit these settings.

Yup, I think that's typical and expected.

Upon running brew rubocop, I get the following output. Not sure if this is an issue on my end or not:

It's not, it's because we're using an old Ruby version and RuboCop is complaining about it.

I did notice that /usr/local/Homebrew/Library/Homebrew/shims/gems/rubocop is consistently slower than just rubocop. Probably because it's invoking brew. Looking at the output of time, the shim is around 0.2 seconds slower, but it was noticeable to me (especially in vscode).

Yeh, that's expected but necessary if we're going to have the same e.g. Ruby logic.

One thing to note: both of these require adding some extensions to vscode. Not sure if we can provide a settings file that includes those or not.

We can 🎉

I've also added a vscode settings file that works for me. Unfortunately, I couldn't figure out how to infer HOMEBREW_REPOSITORY so I hardcoded it as /usr/local/Library/Homebrew/shims/gems/. I tried using $(brew --repository)/Library/Homebrew/shims/gems/ with no luck.

We could try a relative path from the workspace root (or use a workspace variable).

I might be able to take a look at this but if I don't: no worries!

     Why wouldn't this be a developer command, @MikeMcQuaid?

It could be.

As I said, I'm not really sure what the best way to do things here is, so feel free to just revert/drop if what I'm pushing isn't helpful at all.

No, this is really helpful!

A note on this approach: this will likely cause issues if users have their own .vscode directory where they've saved their existing settings since the .vscode directory was ignored.

Yup, thing that's just something we'll need to accept may cause one-off issues.

I also found that if PATH is set to include Library/Homebrew/shims/gems and vscode is launched using the code CLI tool, the default settings will work correctly.

🎉

MikeMcQuaid and others added 3 commits January 7, 2021 13:27
Add a shim and a command that can be used to easily add a single
directory to your `PATH` (`Library/Homebrew/shims/gems`) and have it
automatically install, configure and run `rubocop` so you can use it
for in-editor integrations.
@MikeMcQuaid
Copy link
Copy Markdown
Member Author

I've pushed fixes for all mentioned issues here. @Rylan12 can you see if this still works for you as expected?

Copy link
Copy Markdown
Member

@Rylan12 Rylan12 left a comment

Choose a reason for hiding this comment

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

Awesome! Don't know why I didn't try a relative path...

Anyway, I tested locally and everything appears to work just fine for me. Plus, I learned about the endwise extension (very cool).

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Jan 7, 2021
@BrewTestBot
Copy link
Copy Markdown
Contributor

Review period ended.

@MikeMcQuaid MikeMcQuaid merged commit 3b3d495 into Homebrew:master Jan 8, 2021
@MikeMcQuaid MikeMcQuaid deleted the rubocop-shim branch January 8, 2021 13:04
@MikeMcQuaid
Copy link
Copy Markdown
Member Author

Thanks for the help @Rylan12!

@Rylan12
Copy link
Copy Markdown
Member

Rylan12 commented Jan 8, 2021

Thank you for getting this going!

@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Feb 8, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Feb 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants