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

Unify method for copying special files from repo to site #4601

Merged
merged 9 commits into from Mar 3, 2016

Conversation

Projects
None yet
4 participants
@benbalter
Contributor

benbalter commented Feb 27, 2016

In support of #4596, this pull request introduces a siteify method to the rake tasks, to DRY up copying markdown files from the root to the site (CONDUCT, CONTRIBUTING, and HISTORY).

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Feb 27, 2016

Contributor

Is there any reason to allocate a local instead of just pushing the object directly into the merge?

Contributor

envygeeks commented on Rakefile in 2a2326a Feb 27, 2016

Is there any reason to allocate a local instead of just pushing the object directly into the merge?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Feb 27, 2016

Contributor

Can we add a bit of splitting to that method please? Perhaps use Pathname or Pathutil? Anything that merges the logic of all that File# stuff, including the replacement of extensions.

Contributor

envygeeks commented Feb 27, 2016

Can we add a bit of splitting to that method please? Perhaps use Pathname or Pathutil? Anything that merges the logic of all that File# stuff, including the replacement of extensions.

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter Feb 27, 2016

Contributor

Can we add a bit of splitting to that method please? Perhaps use Pathname or Pathutil? Anything that merges the logic of all that File# stuff, including the replacement of extensions.

@envygeeks I'm afraid I don't follow. Can you elaborate?

Contributor

benbalter commented Feb 27, 2016

Can we add a bit of splitting to that method please? Perhaps use Pathname or Pathutil? Anything that merges the logic of all that File# stuff, including the replacement of extensions.

@envygeeks I'm afraid I don't follow. Can you elaborate?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Feb 27, 2016

Contributor

@benbalter Pathutil can encapsulate everything you did more more cleanly.

file = Pathutil.new(file)
data = file.read
contents = "stuff"

file.sub_ext(".md").write(
  contents
)

As stated before you'll have to manually add pathutils gem to the Jekyll dependencies until my pull to convert all our stuff to Pathutil gets in there but for now we can at least start doing bits and pieces more organized like.

Contributor

envygeeks commented Feb 27, 2016

@benbalter Pathutil can encapsulate everything you did more more cleanly.

file = Pathutil.new(file)
data = file.read
contents = "stuff"

file.sub_ext(".md").write(
  contents
)

As stated before you'll have to manually add pathutils gem to the Jekyll dependencies until my pull to convert all our stuff to Pathutil gets in there but for now we can at least start doing bits and pieces more organized like.

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter Feb 28, 2016

Contributor

@envygeeks started to implement things, and think a refactor like that is better suited for a subsequent pull request to keep this one targeted. The abstraction isn't as clean because we're actually working with two different files.

Contributor

benbalter commented Feb 28, 2016

@envygeeks started to implement things, and think a refactor like that is better suited for a subsequent pull request to keep this one targeted. The abstraction isn't as clean because we're actually working with two different files.

@parkr parkr changed the title from Better file copying to Unify method for copying special files from repo to site Feb 29, 2016

Show outdated Hide outdated History.markdown Outdated
Show outdated Hide outdated Rakefile Outdated
@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 2, 2016

Member

I think it's called History.markdown – will this matching fail?

Member

parkr commented on Rakefile in 7e21d2a Mar 2, 2016

I think it's called History.markdown – will this matching fail?

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter Mar 2, 2016

Contributor

I think it's called History.markdown

Good call. Fixed.

Contributor

benbalter commented Mar 2, 2016

I think it's called History.markdown

Good call. Fixed.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Mar 3, 2016

Member

Thanks, Ben!

@jekyllbot: merge +dev

Member

parkr commented Mar 3, 2016

Thanks, Ben!

@jekyllbot: merge +dev

jekyllbot added a commit that referenced this pull request Mar 3, 2016

@jekyllbot jekyllbot merged commit 7b80cb7 into master Mar 3, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@jekyllbot jekyllbot deleted the copy-better branch Mar 3, 2016

jekyllbot added a commit that referenced this pull request Mar 3, 2016

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