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

Bump Rubocop to 0.54.x #6915

Merged
merged 20 commits into from Apr 9, 2018

Conversation

Projects
None yet
5 participants
@crispgm
Member

crispgm commented Apr 8, 2018

  • Update Gemfile
  • Fix cops

Fix #6655 #6830 #6865
Closes #6626

@DirtyF

This comment has been minimized.

Member

DirtyF commented Apr 8, 2018

@crispgm Do you intend to fix remaining offenses?

https://travis-ci.org/jekyll/jekyll/jobs/363757213#L626

@DirtyF DirtyF added the internal label Apr 8, 2018

@DirtyF DirtyF added this to the 4.0 milestone Apr 8, 2018

@DirtyF

🤖 Please make Rubocop happy again

@crispgm

This comment has been minimized.

Member

crispgm commented Apr 8, 2018

@DirtyF I may have a try. And I wonder why there are even more issues using 0.51.0.

@crispgm crispgm changed the title from Update rubocop 0.54.0 #6865 to WIP: Update rubocop 0.54.0 #6865 Apr 8, 2018

@crispgm

This comment has been minimized.

Member

crispgm commented Apr 8, 2018

@DirtyF there are a lot of empty comments in the codebase. Should I disable it?

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Apr 8, 2018

there are a lot of empty comments in the codebase. Should I disable it?

disable the cop only for /features/

@crispgm crispgm changed the title from WIP: Update rubocop 0.54.0 #6865 to Update rubocop 0.54.0 #6865 Apr 8, 2018

@crispgm

This comment has been minimized.

Member

crispgm commented Apr 8, 2018

@DirtyF Finally, it's happy again! And it's time to make maintainers happy. :)

@DirtyF DirtyF requested a review from jekyll/core Apr 8, 2018

@DirtyF DirtyF dismissed their stale review Apr 8, 2018

Tests are passing

@DirtyF

DirtyF approved these changes Apr 8, 2018

Thanks @crispgm 🍣 🍸

I'd happily update all jekyll core plugins once this is merged.

@DirtyF DirtyF removed this from the 4.0 milestone Apr 8, 2018

@DirtyF DirtyF changed the title from Update rubocop 0.54.0 #6865 to Update rubocop 0.54.0 Apr 8, 2018

@ashmaroli ashmaroli changed the title from Update rubocop 0.54.0 to Bump Rubocop to 0.54.x Apr 8, 2018

@@ -25,6 +25,7 @@ def key?(key)
(@obj.collections.key?(key) && key != "posts") || super
end
# rubocop:disable Naming/MemoizedInstanceVariableName

This comment has been minimized.

@ashmaroli

ashmaroli Apr 8, 2018

Member

Should we just disable this cop globally?

@@ -213,6 +213,7 @@ def generate(site)
@site.process
# exclude files in symlinked directories here and insert them in the
# following step when not on Windows.
# rubocop:disable Lint/NestedPercentLiteral

This comment has been minimized.

@ashmaroli

ashmaroli Apr 8, 2018

Member

IMO this comment ought to be placed at the top level (just before should "sort pages alphabetically" do) and the enabler-comment at the corresponding end..

This comment has been minimized.

@crispgm

crispgm Apr 8, 2018

Member

Are there any best practice on this? Here I disable the minimum scope for the cops.

This comment has been minimized.

@DirtyF

DirtyF Apr 8, 2018

Member

Ideally we don't add any rubocop comments and lists files to exclude from certain cops for now in the rubocop config file and anyone can pick a file and fix it like we did before.

This comment has been minimized.

@crispgm

crispgm Apr 9, 2018

Member

@DirtyF 👌 Done that.

@DirtyF DirtyF requested a review from pathawks Apr 9, 2018

@oe

oe approved these changes Apr 9, 2018

@DirtyF

This comment has been minimized.

Member

DirtyF commented Apr 9, 2018

@jekyllbot: :shipit: +dev

@jekyllbot jekyllbot merged commit 2a032a3 into jekyll:master Apr 9, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@DirtyF DirtyF removed the request for review from pathawks Apr 9, 2018

@crispgm crispgm deleted the crispgm:rubocop-update branch Apr 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment