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

Update Rubocop to 0.51.0 #6444

Merged
merged 1 commit into from Oct 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 1 addition & 3 deletions .rubocop.yml
@@ -1,6 +1,6 @@
---
AllCops:
TargetRubyVersion: 2.0
TargetRubyVersion: 2.1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error: Unsupported Ruby version 2.0 found in `TargetRubyVersion` parameter (in .rubocop.yml). 2.0-compatible analysis was dropped after version 0.50.
Supported versions: 2.1, 2.2, 2.3, 2.4

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jekyll itself dropped support for 2.0. So this is in sync..

Include:
- lib/**/*.rb
Exclude:
Expand Down Expand Up @@ -117,8 +117,6 @@ Style/Documentation:
- !ruby/regexp /features\/.*.rb$/
Style/DoubleNegation:
Enabled: false
Style/Encoding:
EnforcedStyle: when_needed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error: obsolete parameter EnforcedStyle (for Style/Encoding) found in .rubocop.yml
Style/Encoding no longer supports styles. The "never" behavior is always assumed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like us to have the special encoding header on all our files if possible.

Copy link
Member

@ashmaroli ashmaroli Oct 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parkr The Ruby Interpreter from v2.0 onwards encodes all files as UTF-8..

from the Ruby docs:

encoding

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The related Rubocop commit: rubocop/rubocop@5faf140

Style/GuardClause:
Enabled: false
Style/HashSyntax:
Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Expand Up @@ -30,7 +30,7 @@ group :test do
gem "nokogiri", RUBY_VERSION >= "2.2" ? "~> 1.7" : "~> 1.7.0"
gem "rspec"
gem "rspec-mocks"
gem "rubocop", "~> 0.50.0"
gem "rubocop", "~> 0.51.0"
gem "test-dependency-theme", :path => File.expand_path("test/fixtures/test-dependency-theme", __dir__)
gem "test-theme", :path => File.expand_path("test/fixtures/test-theme", __dir__)

Expand Down
6 changes: 3 additions & 3 deletions features/step_definitions.rb
Expand Up @@ -156,7 +156,7 @@
When(%r!^I run jekyll(.*)$!) do |args|
run_jekyll(args)
if args.include?("--verbose") || ENV["DEBUG"]
$stderr.puts "\n#{jekyll_run_output}\n"
warn "\n#{jekyll_run_output}\n"
end
end

Expand All @@ -165,7 +165,7 @@
When(%r!^I run bundle(.*)$!) do |args|
run_bundle(args)
if args.include?("--verbose") || ENV["DEBUG"]
$stderr.puts "\n#{jekyll_run_output}\n"
warn "\n#{jekyll_run_output}\n"
end
end

Expand All @@ -174,7 +174,7 @@
When(%r!^I run gem(.*)$!) do |args|
run_rubygem(args)
if args.include?("--verbose") || ENV["DEBUG"]
$stderr.puts "\n#{jekyll_run_output}\n"
warn "\n#{jekyll_run_output}\n"
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/jekyll/commands/build.rb
Expand Up @@ -96,7 +96,7 @@ def watch(site, options)
)
end
end
end # end of class << self
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/jekyll/commands/build.rb:99:11: C: Style/CommentedKeyword: Do not place comments on the same line as the end keyword.
      end # end of class << self
          ^^^^^^^^^^^^^^^^^^^^^^

end
end
end
2 changes: 1 addition & 1 deletion lib/jekyll/configuration.rb
Expand Up @@ -207,7 +207,7 @@ def read_config_files(files)
rescue ArgumentError => err
Jekyll.logger.warn "WARNING:", "Error reading configuration. " \
"Using defaults (and options)."
$stderr.puts err
warn err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/jekyll/configuration.rb:210:9: C: Style/StderrPuts: Use warn instead of $stderr.puts.
        $stderr.puts err
        ^^^^^^^^^^^^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can these be changed to use Jekyll's logger instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could just append it to the string we are already logging?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisiting this, I'm not sure if we should be changing an outcome of a public method.. (..in the rare case someone's capturing $stderr for some purpose..)

So I think its best that we stick to warn in this PR or disable that check entirely if that's better..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be changing behavior? I'm sure our logger outputs to stderr, right?

end

configuration.fix_common_issues.backwards_compatibilize.add_default_collections
Expand Down
2 changes: 1 addition & 1 deletion lib/jekyll/converters/markdown/redcarpet_parser.rb
Expand Up @@ -47,7 +47,7 @@ def block_code(code, lang)
end

module WithRouge
def block_code(code, lang)
def block_code(_code, lang)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lib/jekyll/converters/markdown/redcarpet_parser.rb:50:20: W: Lint/UnusedMethodArgument: Unused method argument - code. If it's necessary, use _ or _code as an argument name to indicate that it won't be used.
    def block_code(code, lang)
                   ^^^^

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... this looks like a Rubocop bug, as code is clearly used in line 53.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pathawks it doesn't look like a Rubocop bug to me since if you look at line 51, code is being immediately re-assigned with code = "<pre>#{super}</pre>" so in effect overriding whatever has been passed to :block_code as the first argument..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right! It is doing exactly what it’s supposed to do. We don’t need this.

code = "<pre>#{super}</pre>"

"<div class=\"highlight\">#{add_code_tags(code, lang)}</div>"
Expand Down
1 change: 0 additions & 1 deletion test/test_filters.rb
@@ -1,4 +1,3 @@
# coding: utf-8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test/test_filters.rb:1:1: C: Style/Encoding: Unnecessary utf-8 encoding comment.
# coding: utf-8
^^^^^^^^^^^^^^^

# frozen_string_literal: true

require "helper"
Expand Down
1 change: 0 additions & 1 deletion test/test_kramdown.rb
@@ -1,4 +1,3 @@
# encoding: UTF-8
# frozen_string_literal: true

require "helper"
Expand Down
1 change: 0 additions & 1 deletion test/test_tags.rb
@@ -1,4 +1,3 @@
# coding: utf-8
# frozen_string_literal: true

require "helper"
Expand Down
1 change: 0 additions & 1 deletion test/test_utils.rb
@@ -1,4 +1,3 @@
# encoding: utf-8
# frozen_string_literal: true

require "helper"
Expand Down