Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Define path with __dir__ #232

Merged
merged 2 commits into from
Aug 24, 2017
Merged

Define path with __dir__ #232

merged 2 commits into from
Aug 24, 2017

Conversation

DirtyF
Copy link
Member

@DirtyF DirtyF commented Aug 24, 2017

Inspired from jekyll/jekyll#6087 who itself took inspiration from rails/rails@5b8738c

/cc @bogdanvlviv

@DirtyF DirtyF requested review from pathawks and benbalter August 24, 2017 18:36
@@ -15,11 +15,11 @@ class Drop
ENV["JEKYLL_LOG_LEVEL"] = "error"

def dest_dir
File.expand_path("../tmp/dest", File.dirname(__FILE__))
File.expand_path("tmp/dest", __dir__)

Choose a reason for hiding this comment

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

It should be File.expand_path("../tmp/dest", __dir__)

end

def source_dir
File.expand_path("./fixtures", File.dirname(__FILE__))
File.expand_path("./fixtures", __dir__)

Choose a reason for hiding this comment

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

would be nice to get rid of redundant .

Copy link
Collaborator

@benbalter benbalter left a comment

Choose a reason for hiding this comment

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

Huh. TIL'd.

@benbalter benbalter merged commit a5463b1 into master Aug 24, 2017
@benbalter benbalter deleted the define-path-with-__dir__ branch August 24, 2017 18:40
@benbalter
Copy link
Collaborator

Ooop. Sorry @DirtyF @bogdanvlviv got excited by learning something new and moved way too quickly. Glad to address the comments in a follow up PR. Sorry for the itchy finger. 100% my bad.

benbalter added a commit that referenced this pull request Aug 24, 2017
@DirtyF
Copy link
Member Author

DirtyF commented Aug 24, 2017

@bogdanvlviv Thanks for the review.
@benbalter I did push the fixed branch back if needed

@benbalter
Copy link
Collaborator

Since it was my mistake, I implemented both changes via cf90f90. Sorry about that.

@bogdanvlviv
Copy link

bogdanvlviv commented Aug 24, 2017

@benbalter, @DirtyF Nice job! ❤️

@jekyll jekyll locked and limited conversation to collaborators Apr 30, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants