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

Revert Rubocop splat expansion cop #1461

Merged
merged 1 commit into from Mar 24, 2020

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Mar 11, 2020

This PR reverses the change made in #1451. The problem is support for versions of Ruby prior to v2.3. RuboCop ended support for these versions with RuboCop v0.50 but Rouge has not. (The renamed cop was not introduced until RuboCop v0.76.) Dropping support for older versions of Ruby may be implemented as part of Rouge v4.0 but it won't occur as part of v3.0.

@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Mar 11, 2020
@pyrmont pyrmont requested a review from jneen March 11, 2020 00:24
@pyrmont pyrmont self-assigned this Mar 11, 2020
@pyrmont
Copy link
Contributor Author

pyrmont commented Mar 11, 2020

@jneen This problem occurred because of a poor review by me. The change in #1451 passed CI tests and I didn't take the time to check it out locally and confirm. Once approved, I'd like to release as v3.17.1.

@ashmaroli
Copy link
Contributor

@pyrmont A minor correction to the opening post:
It is RuboCop version v0.50 that dropped support for EOL Rubies and RuboCop v0.76 that introduced the renamed cop.

@pyrmont
Copy link
Contributor Author

pyrmont commented Mar 11, 2020

@ashmaroli Corrected, thanks!

@ashmaroli
Copy link
Contributor

@pyrmont You're welcome. But may I ask why this change is considered significant enough to warrant a patch release?
The change is in only a single file which isn't being bundled in the release gem. At max, this can be considered a developmental change, that would only have an affect on contribution to this repository itself.

@pyrmont
Copy link
Contributor Author

pyrmont commented Mar 11, 2020

Yeah, agreed it doesn't require a patch release. Would still like it merged into master as soon as possible, though.

@pyrmont pyrmont removed the request for review from jneen March 24, 2020 19:48
@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Mar 24, 2020
@pyrmont pyrmont merged commit 5f4b818 into rouge-ruby:master Mar 24, 2020
mattt pushed a commit to NSHipster/rouge that referenced this pull request May 21, 2020
This commit reverses the change made in rouge-ruby#1451 and changes the name of
the cop back to `UnneededSplatExpansion`.

The reason to do this is that Rouge supports versions of Ruby from 2.0
and this prevents upgrading our development dependencies to a newer
version of RuboCop (RuboCop ended support for versions prior to v2.3
with RuboCop v0.50). The renamed cop was not introduced until RuboCop 
v0.76.

Dropping support for older versions of Ruby may be implemented as part
of Rouge v4.0 but it won't occur as part of v3.0.
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.

None yet

2 participants