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

[Fix #12557] Make server mode aware of auto-restart for bundle update #13004

Conversation

koic
Copy link
Member

@koic koic commented Jun 18, 2024

Resolves #12557.

Currently, the server mode only auto-restarts due to incompatible versions of RuboCop core. This PR will enable auto-restart when the SHA1 value of Gemfile.lock becomes incompatible. If Gemfile.lock is missing, it will continue to refer to the RuboCop core version as before.

The advantage is that it can cover cases of version incompatibility caused by bundle update, which will likely cover many scenarios like those in #12557.

The disadvantage is that the server mode may become slightly slower. However, it is unlikely to be slow enough for users to notice. The trade-off is considered acceptable.

In the future, it might be possible to detect changes in rubocop configuration as well, but for now, I'd like to see how things go with this change.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@koic koic force-pushed the make_server_mode_aware-of_auto_restart_for_bundle_update branch from 065f2a9 to b477a24 Compare June 18, 2024 17:06
@Earlopain
Copy link
Contributor

Does this actually consider Gemfile.lock? From looking at the code I'd say that file is not used in that check.

…e update`

Resolves rubocop#12557.

Currently, the server mode only auto-restarts due to incompatible versions of RuboCop core.
This PR will enable auto-restart when the SHA1 value of Gemfile.lock becomes incompatible.
If Gemfile.lock is missing, it will continue to refer to the RuboCop core version as before.

The advantage is that it can cover cases of version incompatibility caused by `bundle update`,
which will likely cover many scenarios like those in rubocop#12557.

The disadvantage is that the server mode may become slightly slower.
However, it is unlikely to be slow enough for users to notice. The trade-off is considered acceptable.

In the future, it might be possible to detect changes in rubocop configuration as well, but for now,
I'd like to see how things go with this change.
@koic koic force-pushed the make_server_mode_aware-of_auto_restart_for_bundle_update branch from b477a24 to ed6326c Compare June 18, 2024 17:52
@koic
Copy link
Member Author

koic commented Jun 18, 2024

Ah, indeed. I've updated this PR.

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

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

Fantastic, thank you! 🙏

lib/rubocop/server/cache.rb Show resolved Hide resolved
@bbatsov bbatsov merged commit 4ae9cc6 into rubocop:master Jul 2, 2024
22 checks passed
@koic koic deleted the make_server_mode_aware-of_auto_restart_for_bundle_update branch July 2, 2024 12:33
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.

Wrong version of extensions loaded with 1.59.0?
4 participants