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

Document that _drafts need to be contained within the custom collection directory #6985

Conversation

localheinz
Copy link
Contributor

@localheinz localheinz commented May 4, 2018

This PR

  • adjusts the note about moving the _posts directory into the custom collection directory to also move the _drafts directory

Follows #6680.

πŸ’β€β™‚οΈ Tripped over this, apparently tests for it exist, see

Scenario: Rendered collection in custom collections_dir also containing drafts
Given I have a collections/_puppies directory
And I have the following document under the "puppies" collection within the "collections" directory:
| title | date | content |
| Rover | 2007-12-31 | content for Rover. |
And I have a collections/_drafts directory
And I have the following draft within the "collections" directory:
| title | date | content |
| Gathered Draft | 2009-03-27 | Random Content. |
And I have a "_config.yml" file with content:
"""
collections:
puppies:
output: true
collections_dir: collections
"""
When I run jekyll build --drafts
Then I should get a zero exit status
And the _site directory should exist
And the "_site/puppies/rover.html" file should exist
And I should see "Random Content." in "_site/2009/03/27/gathered-draft.html"
And the _site/collections directory should not exist
Scenario: Rendered collection in custom collections_dir with drafts at the site root
Given I have a collections/_puppies directory
And I have the following document under the "puppies" collection within the "collections" directory:
| title | date | content |
| Rover | 2007-12-31 | content for Rover. |
And I have a _drafts directory
And I have the following draft:
| title | date | content |
| Draft At Root | 2009-03-27 | Random Content. |
And I have a "_config.yml" file with content:
"""
collections:
puppies:
output: true
collections_dir: collections
"""
When I run jekyll build --drafts
Then I should get a zero exit status
And the _site directory should exist
And the "_site/puppies/rover.html" file should exist
And the "_site/2009/03/27/draft-at-root.html" file should not exist
Scenario: A complex site with collections posts and drafts at various locations
Given I have a gathering/_puppies directory
And I have a gathering/_posts directory
And I have a gathering/_drafts directory
And I have a _puppies directory
And I have a _posts directory
And I have a _drafts directory
And I have the following document under the "puppies" collection within the "gathering" directory:
| title | date | content |
| Rover in Gathering | 2007-12-31 | content for Rover. |
And I have the following document under the puppies collection:
| title | date | content |
| Rover At Root | 2007-12-31 | content for Rover. |
And I have the following post within the "gathering" directory:
| title | date | content |
| Post in Gathering | 2009-03-27 | Totally nothing. |
And I have the following post:
| title | date | content |
| Post At Root | 2009-03-27 | Totally nothing. |
And I have the following draft within the "gathering" directory:
| title | date | content |
| Draft In Gathering | 2009-03-27 | This is a draft. |
And I have the following draft:
| title | date | content |
| Draft At Root | 2009-03-27 | This is a draft. |
And I have a "_config.yml" file with content:
"""
collections:
puppies:
output: true
collections_dir: gathering
"""
And I have a "gathering/_puppies/static_file.txt" file that contains "Static content."
And I have a gathering/_puppies/nested directory
And I have a "gathering/_puppies/nested/static_file.txt" file that contains "Nested Static content."
When I run jekyll build --drafts
Then I should get a zero exit status
And the _site directory should exist
And the "_site/puppies/rover-in-gathering.html" file should exist
And the "_site/2009/03/27/post-in-gathering.html" file should exist
And the "_site/2009/03/27/draft-in-gathering.html" file should exist
And the "_site/2009/03/27/draft-at-root.html" file should not exist
And the "_site/puppies/rover-at-root.html" file should not exist
And I should see exactly "Static content." in "_site/puppies/static_file.txt"
And I should see exactly "Nested Static content." in "_site/puppies/nested/static_file.txt"
And the _site/gathering directory should not exist
And the _site/_posts directory should not exist

@localheinz localheinz force-pushed the fix/drafts-with-custom-collection-directory branch 3 times, most recently from 9df2cff to 49107fd Compare May 4, 2018 14:20
Jekyll.logger.warn "",
"Please move '#{posts_at_root}' into the custom directory at " \
"Please move '#{directories.join("', '")}' into the custom directory at " \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this even make sense?

Still doing baby steps in Ruby, apologies when I'm doing it wrong.

πŸ€“

Copy link
Member

Choose a reason for hiding this comment

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

What is the desired output here?

Please move '_drafts, _posts' into the custom directory at..

Copy link
Contributor Author

@localheinz localheinz May 4, 2018

Choose a reason for hiding this comment

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

I think it should be

Please move '_drafts', '_posts' into the custom directory at..

but only if both _drafts and _posts where detected outside the custom collections directory.

Am I doing it wrong? I thought it would be better to combine this into single messages, rather than rendering the same message with different directories. What do you think, @ashmaroli?

Copy link
Member

Choose a reason for hiding this comment

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

To discover (and learn Ruby along the way) the output to your code, you try simple code in your terminal itself..

  • In you terminal, run irb
  • Then execute ["_drafts", "_posts"].join("', '") and see how the output looks like..

Copy link
Member

Choose a reason for hiding this comment

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

Testing for _posts and _drafts separately is the best solution. For that create a new method for _drafts and then extract the common steps between the two into a private helper method..

@ashmaroli
Copy link
Member

@localheinz The maintainers may not release a v3.8.2 with this patch.. So it would be better for the community if you would open a new PR for just fixing the documentation which will go live as soon as that PR is merged..

@localheinz localheinz force-pushed the fix/drafts-with-custom-collection-directory branch 2 times, most recently from 1233181 to 1a4d272 Compare May 4, 2018 14:49
@localheinz localheinz force-pushed the fix/drafts-with-custom-collection-directory branch from 1a4d272 to 3f1edb5 Compare May 4, 2018 15:00
@localheinz
Copy link
Contributor Author

@ashmaroli

Thank you for your feedback! Pulled out the changes to the doctor command into a separate pull request, see #6986.

@ashmaroli ashmaroli changed the title Fix: Remind that _drafts need to be contained in custom collection directory Document that _drafts need to be contained within the custom collection directory May 4, 2018
@ashmaroli ashmaroli force-pushed the fix/drafts-with-custom-collection-directory branch 2 times, most recently from f5fef9c to f360951 Compare May 4, 2018 16:06
@ashmaroli ashmaroli requested a review from a team May 4, 2018 16:09
@DirtyF
Copy link
Member

DirtyF commented May 9, 2018

@jekyllbot: merge +docs

@jekyllbot jekyllbot merged commit 0b196eb into jekyll:master May 9, 2018
jekyllbot added a commit that referenced this pull request May 9, 2018
@jekyll jekyll locked and limited conversation to collaborators Jul 12, 2019
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