-
Notifications
You must be signed in to change notification settings - Fork 487
Fix CI for patches by applying the new standardrb rules #1576
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 CI for patches by applying the new standardrb rules #1576
Conversation
While working on [supporting splatted keyword args](ViewComponent#1560) I discovered that the linter was failing me on changes I didn't make. When I ran standardrb locally, it was quite content, so I was a bit confused. Then I read [.github/workflows/lint.yml] and realized that view_component runs a `bundle update` prior to running standard; which bumps the standardrb version and the corresponding rubocop set. This wasintroduced by @camertron in [e7eec98](ViewComponent@e7eec98) 7+ months ago to fix an error in CI. It's unclear to me why `bundle install` was not working, so I decided that rather than swapping that back I would trust @camertron's instincts and roll-forward with the update. I am not including a change to the Changelog because this seems internal and unnecessary to communicate to package consumers.
I changed it to
I get that, but we'd like every change to have an accompanying changelog entry. I'm currently looking into the PVC breakage 😓 |
|
This PR should fix the PVC tests: #1579 |
If possible, we should omit tooling like standardrb from being updated like that, to prevent the case of it introducing "breaking changes". |
This follows on from [my PR where I applied the updated linting rules](ViewComponent#1576). @camertron made an excellent point that `bundle install` with a checked-in Gemfile.lock results in laggy feedback about breaking changes in upstream packages. And @BlakeWilliams made an excellent point that tooling like `standardrb` is not technically "breaking changes." Theoretically, this should please both Blake and Cameron, as we continue to run `bundle update` in CI as part of the ["Build and test with Rake" github action](https://github.com/ViewComponent/view_component/blob/2cfe54fdd81f8c2f9f79b7d1e4148a2ce3cfdb9c/.github/workflows/ci.yml#L51-L55), while using `bundle install` in the lint action, thus releasing the `standardrb` canary from it's cage. Again, since this is an internal-facing change I didn't want to clutter the Changelog.
This follows on from [my PR where I applied the updated linting rules](#1576). @camertron made an excellent point that `bundle install` with a checked-in Gemfile.lock results in laggy feedback about breaking changes in upstream packages. And @BlakeWilliams made an excellent point that tooling like `standardrb` is not technically "breaking changes." Theoretically, this should please both Blake and Cameron, as we continue to run `bundle update` in CI as part of the ["Build and test with Rake" github action](https://github.com/ViewComponent/view_component/blob/2cfe54fdd81f8c2f9f79b7d1e4148a2ce3cfdb9c/.github/workflows/ci.yml#L51-L55), while using `bundle install` in the lint action, thus releasing the `standardrb` canary from it's cage. Again, since this is an internal-facing change I didn't want to clutter the Changelog.
While working on [supporting splatted keyword args](ViewComponent#1560) I discovered that the linter was failing me on changes I didn't make. When I ran standardrb locally, it was quite content, so I was a bit confused. Then I read [.github/workflows/lint.yml] and realized that view_component runs a `bundle update` prior to running standard; which bumps the standardrb version and the corresponding rubocop set. This wasintroduced by @camertron in [e7eec98](ViewComponent@e7eec98) 7+ months ago to fix an error in CI. It's unclear to me why `bundle install` was not working, so I decided that rather than swapping that back I would trust @camertron's instincts and roll-forward with the update. I am not including a change to the Changelog because this seems internal and unnecessary to communicate to package consumers.
This follows on from [my PR where I applied the updated linting rules](ViewComponent#1576). @camertron made an excellent point that `bundle install` with a checked-in Gemfile.lock results in laggy feedback about breaking changes in upstream packages. And @BlakeWilliams made an excellent point that tooling like `standardrb` is not technically "breaking changes." Theoretically, this should please both Blake and Cameron, as we continue to run `bundle update` in CI as part of the ["Build and test with Rake" github action](https://github.com/ViewComponent/view_component/blob/2cfe54fdd81f8c2f9f79b7d1e4148a2ce3cfdb9c/.github/workflows/ci.yml#L51-L55), while using `bundle install` in the lint action, thus releasing the `standardrb` canary from it's cage. Again, since this is an internal-facing change I didn't want to clutter the Changelog.
While working on [supporting splatted keyword args](ViewComponent#1560) I discovered that the linter was failing me on changes I didn't make. When I ran standardrb locally, it was quite content, so I was a bit confused. Then I read [.github/workflows/lint.yml] and realized that view_component runs a `bundle update` prior to running standard; which bumps the standardrb version and the corresponding rubocop set. This wasintroduced by @camertron in [e7eec98](ViewComponent@e7eec98) 7+ months ago to fix an error in CI. It's unclear to me why `bundle install` was not working, so I decided that rather than swapping that back I would trust @camertron's instincts and roll-forward with the update. I am not including a change to the Changelog because this seems internal and unnecessary to communicate to package consumers.
This follows on from [my PR where I applied the updated linting rules](ViewComponent#1576). @camertron made an excellent point that `bundle install` with a checked-in Gemfile.lock results in laggy feedback about breaking changes in upstream packages. And @BlakeWilliams made an excellent point that tooling like `standardrb` is not technically "breaking changes." Theoretically, this should please both Blake and Cameron, as we continue to run `bundle update` in CI as part of the ["Build and test with Rake" github action](https://github.com/ViewComponent/view_component/blob/2cfe54fdd81f8c2f9f79b7d1e4148a2ce3cfdb9c/.github/workflows/ci.yml#L51-L55), while using `bundle install` in the lint action, thus releasing the `standardrb` canary from it's cage. Again, since this is an internal-facing change I didn't want to clutter the Changelog.
What are you trying to accomplish?
CI is failing on lint for everyone, regardless of changes. This gets through that in a small, temporary way.
While working on supporting splatted keyword
args I discovered that the linter was failing me on changes I didn't make.
When I ran standardrb locally, it was quite content, so I was a bit confused. Then I read [.github/workflows/lint.yml] and realized that view_component runs a
bundle updateprior to running standard; which bumps the standardrb version and the corresponding rubocop set.This was introduced by @camertron in
e7eec9869fc13e401401c687573a98f1b0ed57a8 7+ months ago to fix an error in CI.
What approach did you choose and why?
It's unclear to me why
bundle installwas not working, so I decided that rather than swapping that back I would trust @camertron's instincts and roll-forward by updating theGemfile.lockso folks' local matches CI (for now), and applying the changes it recommended.I am not including a change to the Changelog because this seems internal and unnecessary to communicate to package consumers.
Anything you want to highlight for special attention from reviewers?
I'd be down to just try
bundle installrather thanbundle updateif that seems like a good idea to other folks with more context.