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

rubocop: lib/jekyll/regenerator.rb #5025

Merged
merged 2 commits into from Jun 23, 2016

Conversation

Projects
None yet
3 participants
@ayastreb
Contributor

ayastreb commented Jun 19, 2016

#4885

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 21, 2016

Member

/cc @alfredxing whose 👀 I'd love on this because it's all regeneration code 😄

Member

parkr commented Jun 21, 2016

/cc @alfredxing whose 👀 I'd love on this because it's all regeneration code 😄

source_path = document.respond_to?(:path) ? document.path : nil
dest_path = if document.respond_to?(:destination)
document.destination(@site.dest)
end

This comment has been minimized.

@parkr

parkr Jun 21, 2016

Member

What was wrong with the previous two lines? Just too long?

It seems like your solution for source_path and for dest_path are two ways of doing the same thing.

@parkr

parkr Jun 21, 2016

Member

What was wrong with the previous two lines? Just too long?

It seems like your solution for source_path and for dest_path are two ways of doing the same thing.

This comment has been minimized.

@ayastreb

ayastreb Jun 22, 2016

Contributor

Yes, dest_path line was 98 chars long.
Is there another way to make it shorter?)

@ayastreb

ayastreb Jun 22, 2016

Contributor

Yes, dest_path line was 98 chars long.
Is there another way to make it shorter?)

@jekyllbot jekyllbot removed the needs-work label Jun 22, 2016

@ayastreb

This comment has been minimized.

Show comment
Hide comment
@ayastreb

ayastreb Jun 22, 2016

Contributor

Hi @parkr,
I've reworked modified? method a bit, hope now it's more clear 😄

Contributor

ayastreb commented Jun 22, 2016

Hi @parkr,
I've reworked modified? method a bit, hope now it's more clear 😄

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 23, 2016

Member

This looks GREAT! Well done, thank you ❤️

@jekyllbot: merge +dev

Member

parkr commented Jun 23, 2016

This looks GREAT! Well done, thank you ❤️

@jekyllbot: merge +dev

@jekyllbot jekyllbot merged commit d093c76 into jekyll:master Jun 23, 2016

1 check passed

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

@parkr parkr added the refactor label Jun 23, 2016

jekyllbot added a commit that referenced this pull request Jun 23, 2016

@ayastreb ayastreb deleted the ayastreb:regenerator branch Jun 23, 2016

parkr added a commit that referenced this pull request Jun 29, 2016

Merge branch 'master' into remove-jruby-and-ruby-head
* master: (41 commits)
  Fix rubocop offenses on master.
  script/fmt: print Rubocop version
  Update history to reflect merge of #5030 [ci skip]
  rubocop: separate deprecator error messages
  Update history to reflect merge of #5031 [ci skip]
  Update history to reflect merge of #5032 [ci skip]
  rubocop: fix code style
  rubocop: fix code style
  rubocop: fix code style
  Update history to reflect merge of #5024 [ci skip]
  Update history to reflect merge of #5025 [ci skip]
  utils: check that the object is a hash when merging default_proc
  Update history to reflect merge of #5026 [ci skip]
  rubocop: refactor modified? method
  Add a benchmark for capture vs. assign in Liquid. [ci skip]
  Update history to reflect merge of #5027 [ci skip]
  Add generator-jekyllized to third party plugins
  rubocop: fix code style
  rubocop: fix code style
  rubocop: fix code style
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment