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

Ruby 2.3 cleanup #8056

Merged
merged 5 commits into from May 29, 2020
Merged

Ruby 2.3 cleanup #8056

merged 5 commits into from May 29, 2020

Conversation

@bquorning
Copy link
Member

bquorning commented May 29, 2020

Since we dropped support for Ruby 2.3 in 5873894, there’s some cleanup to be done:

  • Documentation should not refer to Ruby 2.3.
  • Code comments should not refer to Ruby 2.3.
  • Layout/HeredocIndentation cop should not allow active_support, powerpack and unindent styles, only the squiggly style.

Those three issues are fixed by this PR.

Please note that after removing functionality from Layout/HeredocIndentation I couldn’t resist cleaning up the remaining code and specs. Those changes may be deemed large enough for a separate pull request. Please let me know.


Before submitting the PR make sure the following are checked:

  • 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.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.
@bquorning bquorning force-pushed the bquorning:ruby-2.3-cleanup branch from 9e63fea to 28ccb1e May 29, 2020
@bquorning bquorning requested a review from koic May 29, 2020
@bquorning bquorning marked this pull request as ready for review May 29, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented May 29, 2020

The changes seems good to me overall, but I'm confused why did you modify the legacy markdown docs instead of the new docs.

@bbatsov
Copy link
Collaborator

bbatsov commented May 29, 2020

Btw, I didn't delete the markdown site only to avoid breaking links to it. At some point we'll have to redirect somehow from the RTD to the new pages.

bquorning added 5 commits May 29, 2020
Following the removal of support for Ruby 2.3 in 5873894,
there is no reason for Layout/HeredocIndentation to recomment using
anything other than the "squiggly heredoc" <<~.
Support for Ruby 2.3 was dropped in 5873894.
@bquorning bquorning force-pushed the bquorning:ruby-2.3-cleanup branch from 28ccb1e to 8a3c323 May 29, 2020
@bquorning
Copy link
Member Author

bquorning commented May 29, 2020

The changes seems good to me overall, but I'm confused why did you modify the legacy markdown docs instead of the new docs.

Ahh, sorry. I ran rake before realising I had to rebase on master. I’ll have force-pushed with the real doc changes.

So the entire manual folder is going away? If it’s not supposed to be kept updated, shouldn’t it be deleted? Update: I just saw your 2nd comment :-)

@bbatsov
Copy link
Collaborator

bbatsov commented May 29, 2020

The problem is when you Google for RuboCop docs now you get the following:

image

If we just delete the entire folder without setting up some redirects that's going to break the top search results. I guess that's not that big of a deal, but we should probably avoid it.

@bquorning
Copy link
Member Author

bquorning commented May 29, 2020

we should probably avoid it.

Good point.

expect_offense(annotated_source)

corrected = looped_autocorrect_from_annotated_source(annotated_source)
expect(corrected).to eq(<<~CORRECTION)

This comment has been minimized.

Copy link
@bquorning

bquorning May 29, 2020

Author Member

It would actually be pretty neat if you could pass a loop: true 2nd argument to expect_correction. I noticed a few other places in the specs that rely on multiple passes when testing autocorrection.

This comment has been minimized.

Copy link
@bbatsov

bbatsov May 29, 2020

Collaborator

You can open a separate ticket to track this idea.

@bbatsov bbatsov merged commit 2a037fc into rubocop-hq:master May 29, 2020
26 checks passed
26 checks passed
windows 2.4
Details
windows 2.5
Details
windows 2.6
Details
windows 2.7
Details
windows mingw
Details
ci/circleci: cc-setup Your tests passed on CircleCI!
Details
ci/circleci: cc-upload-coverage Your tests passed on CircleCI!
Details
ci/circleci: documentation-checks Your tests passed on CircleCI!
Details
ci/circleci: jruby-9.2-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: jruby-9.2-rubocop Your tests passed on CircleCI!
Details
ci/circleci: jruby-9.2-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.4-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.4-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.4-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.5-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.5-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.5-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.6-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.6-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.6-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.7-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.7-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-2.7-spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-head-ascii_spec Your tests passed on CircleCI!
Details
ci/circleci: ruby-head-rubocop Your tests passed on CircleCI!
Details
ci/circleci: ruby-head-spec Your tests passed on CircleCI!
Details
@bquorning bquorning deleted the bquorning:ruby-2.3-cleanup branch May 29, 2020
@sergey-alekseev
Copy link
Contributor

sergey-alekseev commented Jun 3, 2020

I'm confused why did you modify the legacy markdown docs instead of the new docs

Actually it was confusing

  1. getting the warning (see the comment footer),
  2. going to https://rubocop.readthedocs.io/en/stable/cops_layout/#layoutheredocindentation (by googling or remembering that site),
  3. finding out there is still EnforcedStyle

In my view having both websites should be modified until

At some point we'll have to redirect somehow from the RTD to the new pages.

Now I see there is https://docs.rubocop.org/rubocop/0.85/cops_layout.html#layoutheredocindentation, but I found it only after going to the changelog, searching for 'Layout/HeredocIndentation' and finding this PR. There is no docs.rubocop.org among my search results:
Screen Shot 2020-06-03 at 10 54 33


Initially I encountered the following warning and had to found this PR. Leaving this to be indexed by search bots and potentially help other people:

Warning: Layout/HeredocIndentation does not support EnforcedStyle parameter.

Supported parameters are:

  - Enabled
sergey-alekseev added a commit to sergey-alekseev/ruby-style-guide that referenced this pull request Jun 3, 2020
See rubocop-hq/rubocop#8056 (comment). Actual since 804d615. From https://github.com/Shopify/ruby-style-guide/runs/732692117:
```
Warning: Layout/HeredocIndentation does not support EnforcedStyle parameter.

Supported parameters are:

  - Enabled

Warning: Layout/HeredocIndentation does not support EnforcedStyle parameter.

Supported parameters are:

  - Enabled
```
tas50 added a commit to tas50/chef-2 that referenced this pull request Jul 16, 2020
When Ruby 2.3 support was dropped in RuboCop they changed the Layout/HeredocIndentation to only support squiggly so setting this just causes it to warn. Just enable it instead.

See rubocop-hq/rubocop#8056

Signed-off-by: Tim Smith <tsmith@chef.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.