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

Release v3.7.1 #6695

Merged
merged 4 commits into from Jan 25, 2018
Merged

Release v3.7.1 #6695

merged 4 commits into from Jan 25, 2018

Conversation

ashmaroli
Copy link
Member

@ashmaroli ashmaroli commented Jan 15, 2018

@ashmaroli ashmaroli requested a review from a team January 15, 2018 09:22
@ashmaroli ashmaroli added the release Time to cut a new gem! label Jan 15, 2018
Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

Do we measure the consequences of moving posts in collection_dir for plugins?

of the said directory. <br />
We addressed this by having Jekyll scan the directory path only if the user
explicitly configures the `scope["path"]` using wildcards. <br />
Read our documentation for more details. <br />
Copy link
Member

Choose a reason for hiding this comment

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

Link ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Working on a document to attach to the associated PR.. will update after it gets merged..

Copy link
Member

Choose a reason for hiding this comment

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

Are all the <br /> tags necessary? Can't we use Markdown line breaks?

Copy link
Member Author

Choose a reason for hiding this comment

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

Markdown (or GFM, not sure..) converts \n\s* into \s and \n\n into \n<p>
I wanted to just insert simple breaks.. I'll locally "preview" the document once more and adopt the better outcome..

Copy link
Member

Choose a reason for hiding this comment

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

Link to the docs is still missing here.


Close on the heels of shipping 3.7.0, we were informed of a couple of
regressions due to the changes made in that release. In due time, Team Jekyll
set out to address those issues as early as possible.
Copy link
Member

Choose a reason for hiding this comment

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

Link to Jekyll teams ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was referring to the team of maintainers instead of a particular team..
so do you want me to link to affinity-team-captains here?

Copy link
Member

Choose a reason for hiding this comment

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

As Jekyll need more contributors, linking to teams is meant as a way of reminding that people are welcome to join.

Copy link

Choose a reason for hiding this comment

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

isn't Team Jekyll meant kind of tongue-in-cheek here? at least that's how i interpreted it, but a link to jekyll teams would be nice, agreed

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not intend it to be tongue-in-cheek at all..:
I just meant to refer to the maintainers of the codebase without focusing on a particular team or set of individuals..

Props to @localheinz for asserting this regression to our notice. <br />
We decided to resolve this by having Jekyll ignore posts and drafts at the
root of the site's source directory *if the user customizes the
`collection_dir` setting.* <br />
Copy link
Member

Choose a reason for hiding this comment

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

Moving _posts and _draftsout of the source directory is a tempting fix but can't it have consequences on how some plugins work ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd love it if you can point me to at least one popular plugin that could break.. (not a challenge.., I'd really like to see if there's a workaround / resolution..)

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking of paginationv2, jekyll-admin, jekyll-feed to name a few

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC, jekyll-feed, jekyll-sitemap calls to posts via {% for post in site.posts %}. Since Document attributes do not change at all, (relative_path is the same irrespective of the collection_dir) setting, I doubt these plugins are going to break..
But what is evident from jekyll/jekyll-feed#202 is that plugins will break if the posts are at the root directory when a custom collection_dir has been set.

@pathawks
Copy link
Member

I think #6685 is minor rather than bug-fix

This should be 3.8.0?

@ashmaroli
Copy link
Member Author

#6685 is a minor ? Its not introducing any new features / public_methods.. just fixing stuff that broke in 3.7.0..

@pathawks
Copy link
Member

It requires moving _posts/. Perhaps it is a bug fix, but it might require user intervention and I’d rather err on the side of letting users know when things may have changed.

@ashmaroli
Copy link
Member Author

ashmaroli commented Jan 16, 2018

Things actually changed in v3.7.0.. User intervention is required (to customize the collections_dir setting) on v3.7.0 and master even..
./_posts/2018-01-15-test-post.md will get generated into ./_site/_posts/2018/01/15/test-post.html with a custom collections_dir even on master! (which is the bug actually..)

The PR is just normalizing things again.. mitigating confusion....

@pathawks
Copy link
Member

I follow. In that case, I’m on board with bug-fix 👍

Thanks for taking the time to explain 🍻

regressions due to the changes made in that release. In due time, Team Jekyll
set out to address those issues as early as possible.

Days later, here we're announcing 3.7.1 that aims to set things in order. The
Copy link

Choose a reason for hiding this comment

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

unneccessary use of here

Copy link

Choose a reason for hiding this comment

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

also: can you replace set things in order with set things right? i think it's a more common idiom

Copy link
Member Author

Choose a reason for hiding this comment

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

here in ln#13 is an extension to the previous sentence..

A week ago, we set out to vanquish our enemies.. And now, here we're, breaking bread with them..

Copy link

Choose a reason for hiding this comment

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

fair enough!

that directory for any subfolders and files, for each document in that
`path`.
Though this is intended, it increases build times in proportion to the size
of the said directory. <br />
Copy link

Choose a reason for hiding this comment

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

said can be removed

explicitly configures the `scope["path"]` using wildcards. <br />
Read our documentation for more details. <br />
A huge shout-out to @mmistakes for bringing this to our notice and
additionally providing with a test repository to aid in resolving the issue.
Copy link

Choose a reason for hiding this comment

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

providing us

Users setting `collection_dir` to a certain directory would have *altered*
paths to their posts still at the root of their site's source. This
roughly translated to 404 errors on URLs to their posts. <br />
Props to @localheinz for asserting this regression to our notice. <br />
Copy link

Choose a reason for hiding this comment

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

i think replacing asserting with bringing is more readable

have been made that should improve your Jekyll experience. All of which would
not have been possible without our wonderful contributors: Andreas Möller,
Ashwin Maroli, Florian Thomas, Frank Taillandier, Olivia and Parker Moore.

Copy link
Member

Choose a reason for hiding this comment

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

Update to : Alexandr, Andreas Möller, Ashwin Maroli, Florian Thomas, Frank Taillandier, Kacper Duras, Olivia, Parker Moore, Paul Robert Lloyd

of the said directory. <br />
We addressed this by having Jekyll scan the directory path only if the user
explicitly configures the `scope["path"]` using wildcards. <br />
Read our documentation for more details. <br />
Copy link
Member

Choose a reason for hiding this comment

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

Link to the docs is still missing here.


* A major regression in 3.7.0 was that when a Front Matter Default was
configured with a `scope["path"]` set to a directory, Jekyll would scan
that directory for any subfolders and files, for each document in that
`path`.
Though this is intended, it increases build times in proportion to the size
of the said directory. <br />
of the said.
Copy link

Choose a reason for hiding this comment

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

of the directory

Copy link
Member Author

Choose a reason for hiding this comment

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

oops! 😁

@ashmaroli ashmaroli changed the title WIP: Release v3.7.1 Release v3.7.1 Jan 25, 2018
@ghost
Copy link

ghost commented Jan 25, 2018

@jekyllbot: merge +docs

@jekyllbot jekyllbot merged commit 5ca193e into jekyll:master Jan 25, 2018
jekyllbot added a commit that referenced this pull request Jan 25, 2018
@ashmaroli ashmaroli deleted the release-3-7-1 branch January 29, 2018 16:51
@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.
Labels
frozen-due-to-age release Time to cut a new gem!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants