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

Replace nested conditional with guard clauses #8294

Conversation

torrocus
Copy link
Contributor

@torrocus torrocus commented Jul 9, 2020

This is refactoring.

Summary

I wanted to remove the lib/jekyll/tags/include.rb file from the exclusion list in .rubocop_todo.yml.
But when I looked there, I did a little refactoring.
Now this code is more readable.

Context

It isn't related to any GitHub issue.
The functionality is the same.

@ashmaroli
Copy link
Member

If you're willing to, the first two lines could be squashed as well:

- page = context.registers[:page]
- site = context.registers[:site]
+ page, site = context.registers.values_at(:page, :site)

@torrocus
Copy link
Contributor Author

torrocus commented Jul 9, 2020

- page = context.registers[:page]
- site = context.registers[:site]
+ page, site = context.registers.values_at(:page, :site)

It makes sense. We don't need to refer to twice context.registers. Done.

Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

LGTM!

@ashmaroli ashmaroli requested a review from a team July 9, 2020 17:05
Copy link
Member

@pathawks pathawks left a comment

Choose a reason for hiding this comment

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

❤️

@ashmaroli
Copy link
Member

@jekyllbot: merge +fix

@jekyllbot jekyllbot merged commit f5826ee into jekyll:master Jul 9, 2020
@ashmaroli
Copy link
Member

Thank you @torrocus

Copy link

@bamaswade bamaswade left a comment

Choose a reason for hiding this comment

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

site.in_source_dir

@@ -252,21 +252,21 @@ def tag_includes_dirs(context)
Array(page_path(context)).freeze
end

private

Choose a reason for hiding this comment

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

Ok

Copy link

@bamaswade bamaswade left a comment

Choose a reason for hiding this comment

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

lib/jekyll/tags/include.rb

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants