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

Allow Jekyll Doctor to detect stray posts dir #6681

Merged
merged 2 commits into from Feb 20, 2018

Conversation

Projects
None yet
6 participants
@ashmaroli
Member

ashmaroli commented Jan 7, 2018

If one defines a custom location for their collections, _posts directory should not exist at the root of the source directory

@DirtyF

DirtyF approved these changes Jan 7, 2018

👍 excellent idea, even better of we can give people a clear instruction.

return true unless File.directory?(posts_at_root)
Jekyll.logger.warn "Warning:", "Contents of #{posts_at_root} will not be " \
"processed since you have specified a custom directory " \
"to house all collections."

This comment has been minimized.

@DirtyF

DirtyF Jan 7, 2018

Member

Could we rater display the awaited action from the user?

Warning: Custom collection_dir detected : move the _posts directory inside the my_collections directory. (where my_collections is the option set by the user)

@localheinz

This comment has been minimized.

Contributor

localheinz commented Jan 7, 2018

@ashmaroli

This is awesome, and I'm sure much better suited to save people from headaches, hehe!

@DirtyF

DirtyF approved these changes Jan 7, 2018

@DirtyF DirtyF requested a review from jekyll/build Jan 7, 2018

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 7, 2018

I'm sure much better suited to save people from headaches

@localheinz only if they make it a point to run jekyll doctor when something's not right.. 😄
Besides, this won't reach our users until next minor release.. Documentation updates are go live instantly.. 😃

@Crunch09

👍

@DirtyF

This comment has been minimized.

Member

DirtyF commented Jan 7, 2018

What about an option jekyll build --check that would run jekyll doctor before build?

@ashmaroli

This comment has been minimized.

Member

ashmaroli commented Jan 7, 2018

What about an option jekyll build --check that basically run jekyll doctor before build?

Comparing

def process(options)
site = Jekyll::Site.new(configuration_from_options(options))
site.reset
site.read
site.generate

and
def process
reset
read
generate
render
cleanup
write
print_stats if config["profile"]
end

the first three steps of the "process" is the same..

So it has to be more of a split-action procedure instead of a before-and-after..
Nice idea nevertheless.. 👍

@ashmaroli ashmaroli added this to the v3.8.0 milestone Jan 19, 2018

@oe oe referenced this pull request Feb 17, 2018

Closed

Release 3.8.0 #6783

12 of 13 tasks complete
@oe

This comment has been minimized.

Member

oe commented Feb 20, 2018

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit cef66de into jekyll:master Feb 20, 2018

3 checks passed

WIP ready for review
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ashmaroli ashmaroli deleted the ashmaroli:assess-custom-coll-dir branch Feb 20, 2018

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