Rubocop cleanup for some utils and further test files #4916

Merged
merged 10 commits into from May 23, 2016

Conversation

Projects
None yet
4 participants
@brint
Contributor

brint commented May 20, 2016

More cleanup for #4885

Files addressed:

lib/jekyll/utils/ansi.rb
lib/jekyll/utils/platforms.rb
lib/jekyll/version.rb
test/test_related_posts.rb
test/test_sass.rb
test/test_static_file.rb
test/test_theme.rb
test/test_url.rb

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks May 20, 2016

Contributor

You can skip everything inside of ansi.rb recenly Simple::Ansi and Colorator merged and everything in that file will land in Jekyll once we upgrade Colorator so it'll probably be dropped in favor of just using our dependency which has way more stuff and is managed independently.

Contributor

envygeeks commented May 20, 2016

You can skip everything inside of ansi.rb recenly Simple::Ansi and Colorator merged and everything in that file will land in Jekyll once we upgrade Colorator so it'll probably be dropped in favor of just using our dependency which has way more stuff and is managed independently.

lib/jekyll/version.rb
@@ -1,3 +1,3 @@
module Jekyll
- VERSION = '3.1.6'
+ VERSION = "3.1.6".freeze

This comment has been minimized.

@envygeeks

envygeeks May 20, 2016

Contributor

Anything to do with freezing strings can be skipped, there are plans to add the magic comments soon.

@envygeeks

envygeeks May 20, 2016

Contributor

Anything to do with freezing strings can be skipped, there are plans to add the magic comments soon.

This comment has been minimized.

@brint

brint May 20, 2016

Contributor

Not sure what to do here. Revert the change and wait?

@brint

brint May 20, 2016

Contributor

Not sure what to do here. Revert the change and wait?

This comment has been minimized.

@parkr

parkr May 20, 2016

Member

Leave as-is. When we do the magic comment for 2.3.0 and we drop support for <= 2.3.0, we'll remove this 😄

@parkr

parkr May 20, 2016

Member

Leave as-is. When we do the magic comment for 2.3.0 and we drop support for <= 2.3.0, we'll remove this 😄

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks May 20, 2016

Contributor

Awesome, thanks for helping out!

Contributor

envygeeks commented May 20, 2016

Awesome, thanks for helping out!

@brint

This comment has been minimized.

Show comment
Hide comment
@brint

brint May 20, 2016

Contributor

Would you like the ansi.rb changes reverted? I can do that. It was in the list for #4885.

Contributor

brint commented May 20, 2016

Would you like the ansi.rb changes reverted? I can do that. It was in the list for #4885.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks May 20, 2016

Contributor

@brint nah that would be a waste of your effort, that was more of a note for anybody else.

Contributor

envygeeks commented May 20, 2016

@brint nah that would be a waste of your effort, that was more of a note for anybody else.

test/test_static_file.rb
- static_file = setup_static_file_with_collection(
- "root", "_foo/dir/subdir", "file.html", "foo", {"output" => true})
+ static_file = setup_static_file_with_collection("root", "_foo/dir/subdir",
+ "file.html", { "output" => true })

This comment has been minimized.

@parkr

parkr May 20, 2016

Member

@brint Let's put each of these arguments on its own line:

static_file = setup_static_file_with_collection(
  "root",
  "_foo/dir/subdir",
  "file.html",
  { "output" => true }
)
@parkr

parkr May 20, 2016

Member

@brint Let's put each of these arguments on its own line:

static_file = setup_static_file_with_collection(
  "root",
  "_foo/dir/subdir",
  "file.html",
  { "output" => true }
)
test/test_static_file.rb
- {"output" => true, "permalink" => "/:path/"})
+ should "use its collection's permalink template for destination relative directory" do
+ static_file = setup_static_file_with_collection("root", "_foo/dir/subdir",
+ "file.html", { "output" => true, "permalink" => "/:path/" })

This comment has been minimized.

@parkr

parkr May 20, 2016

Member

Multiple lines here too:

static_file = setup_static_file_with_collection(
  "root",
  "_foo/dir/subdir",
  "file.html",
  { "output" => true, "permalink" => "/:path/" }
)
@parkr

parkr May 20, 2016

Member

Multiple lines here too:

static_file = setup_static_file_with_collection(
  "root",
  "_foo/dir/subdir",
  "file.html",
  { "output" => true, "permalink" => "/:path/" }
)
test/test_static_file.rb
- static_file = setup_static_file_with_defaults(
- "root", "private/dir/subdir", "file.html", defaults)
+ static_file = setup_static_file_with_defaults("root", "private/dir/subdir",
+ "file.html", defaults)

This comment has been minimized.

@parkr

parkr May 20, 2016

Member

And here:

static_file = setup_static_file_with_collection(
  "root",
  "private/dir/subdir",
  "file.html",
  defaults
)
@parkr

parkr May 20, 2016

Member

And here:

static_file = setup_static_file_with_collection(
  "root",
  "private/dir/subdir",
  "file.html",
  defaults
)
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr May 20, 2016

Member

This is great! Just a couple comments above. Thank you for doing this, @brint!

Member

parkr commented May 20, 2016

This is great! Just a couple comments above. Thank you for doing this, @brint!

@parkr parkr referenced this pull request May 20, 2016

Closed

Make jekyll/jekyll code compliant with rubocop rules #4885

114 of 115 tasks complete
@brint

This comment has been minimized.

Show comment
Hide comment
@brint

brint May 21, 2016

Contributor

@parkr Thanks for the feedback. The args have been split onto their own lines.

Contributor

brint commented May 21, 2016

@parkr Thanks for the feedback. The args have been split onto their own lines.

@parkr parkr changed the title from Rubocop cleanup for #4885 to Rubocop cleanup for some utils and further test files May 23, 2016

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr May 23, 2016

Member

Thanks again, @brint! You're on a roll!

@jekyllbot: merge +dev

Member

parkr commented May 23, 2016

Thanks again, @brint! You're on a roll!

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit 6eaf263 into jekyll:master May 23, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@brint brint deleted the brint:rubocop_cleaning branch May 23, 2016

parkr added a commit that referenced this pull request May 23, 2016

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