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.

Sign up for GitHub

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

Jump to bottom

fix date parsing in file names to be stricter #5609

Merged
merged 4 commits into from Nov 29, 2016
Merged

fix date parsing in file names to be stricter #5609

merged 4 commits into from Nov 29, 2016

Conversation

ghost
Copy link

@ghost ghost commented Nov 29, 2016

this fixes #5603

ghost
ghost commented Nov 29, 2016
View reviewed changes
test/test_document.rb
@@ -492,4 +492,33 @@ def assert_equal_value(key, one, other)
assert_equal true, File.file?(@dest_file)
end
end

context "a document in a collection with dash-separated numeric file name" do
Copy link
Author

Choose a reason for hiding this comment

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

we don't really need these tests because most others in this spec file will fail if the regex is wrong, but i added them for consistency

@ashmaroli
Copy link
Member

Before this pull, the expected @site.posts.size was 51 in test/test_generated_site.rb
With this pull, a new file was added to /methods collection ( albeit ignored by Jekyll ) yet @site.posts.size fell to a 49 ..
which other files got ignored??

@ghost
Copy link
Author

ghost commented Nov 29, 2016

@ashmaroli okay, that is weird. i'll have a look

@ashmaroli
Copy link
Member

@fene these two files got ignored with this pull:

category/_posts/2008-9-23-categories.markdown
z_category/_posts/2008-9-23-categories.markdown

They were ignored because they have 9 instead of 09..

This could potentially break some existing sites.. so a documentation-update or a deprecation-release is warranted..

@ghost
Copy link
Author

ghost commented Nov 29, 2016

ping @jekyll/core @jekyll/documentation what do you think about this?

@ashmaroli
Copy link
Member

I guess we're covered documentation-wise.

posts

and that section has been around for years now

@ghost
Copy link
Author

ghost commented Nov 29, 2016

^ weird travis error on the latest build

@ashmaroli
Copy link
Member

Don't worry about it.. its a bug.. occurs randomly..
it'll go away when one of the maintainers will restart that lone test for you..

parkr
parkr approved these changes Nov 29, 2016
View reviewed changes
Copy link
Member

@parkr parkr left a comment

Choose a reason for hiding this comment

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

Nicely done, @fene!

@parkr parkr changed the title fix date parsing in file names fix date parsing in file names to be stricter Nov 29, 2016
@parkr
Copy link
Member

parkr commented Nov 29, 2016

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit c8c6c6c into jekyll:master Nov 29, 2016
jekyllbot added a commit that referenced this pull request Nov 29, 2016
@jekyllbot
Update history to reflect merge of #5609 [ci skip]
35c5e07
parkr pushed a commit that referenced this pull request Dec 16, 2016
@jekyllbot @parkr
Update history to reflect merge of #5609 [ci skip]
d97f5cb
parkr added a commit that referenced this pull request Mar 2, 2017
@parkr
Failing test: abbreviated post dates are no longer read.
8a0c072 
This is a regression introduced by #5609.
@parkr parkr mentioned this pull request Mar 2, 2017
Merged
@jekyll jekyll locked and limited conversation to collaborators Jul 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@DirtyF DirtyF

Labels
fix frozen-due-to-age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jekyll parses any dash-separated numeric filename as date
4 participants
@ashmaroli @parkr @DirtyF @jekyllbot