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

Backward compatiblize URLFilters module #6163

Merged
merged 1 commit into from Jun 21, 2017

Conversation

Projects
None yet
7 participants
@ashmaroli
Member

ashmaroli commented Jun 20, 2017

#6058 introduced a private method :site to Jekyll::Filters::URLFilters that created unforeseen bugs when the module got extended. This PR reverts that private_method to oblivion.

/cc @jekyll/ecosystem @benbalter

@parkr parkr requested a review from benbalter Jun 20, 2017

@DirtyF

DirtyF approved these changes Jun 21, 2017

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jun 21, 2017

Member

@jekyllbot: merge +bug

Member

pathawks commented Jun 21, 2017

@jekyllbot: merge +bug

@jekyllbot jekyllbot merged commit 7a85376 into jekyll:master Jun 21, 2017

2 checks passed

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

@jekyllbot jekyllbot added bug fix labels Jun 21, 2017

jekyllbot added a commit that referenced this pull request Jun 21, 2017

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jun 21, 2017

Member

@parkr It'd be cool to get this fix out quickly.

@jekyll/ecosystem I wonder if we could do a better job testing Jekyll with some popular plugins before we release, just to make sure we aren't unknowingly breaking anything major the way this did. Breaking most (all?) of our own plugins doing a point update is not a great look.

Member

pathawks commented Jun 21, 2017

@parkr It'd be cool to get this fix out quickly.

@jekyll/ecosystem I wonder if we could do a better job testing Jekyll with some popular plugins before we release, just to make sure we aren't unknowingly breaking anything major the way this did. Breaking most (all?) of our own plugins doing a point update is not a great look.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jun 21, 2017

Member

@ashmaroli Thank you for fixing this so quickly 🍻♥️

Member

pathawks commented Jun 21, 2017

@ashmaroli Thank you for fixing this so quickly 🍻♥️

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 21, 2017

Member

@parkr It'd be cool to get this fix out quickly.

Happy to release if someone wants to put together a release post.

Member

parkr commented Jun 21, 2017

@parkr It'd be cool to get this fix out quickly.

Happy to release if someone wants to put together a release post.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jun 21, 2017

Member

@ashmaroli?

Otherwise, I’d be happy to do so tonight.

Member

pathawks commented Jun 21, 2017

@ashmaroli?

Otherwise, I’d be happy to do so tonight.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Jun 22, 2017

Member

@pathawks Thank you. But, I'll let you do the honors.
Before we release, have we tested the failing plugins against the master branch of this repo to be sure..?

Member

ashmaroli commented Jun 22, 2017

@pathawks Thank you. But, I'll let you do the honors.
Before we release, have we tested the failing plugins against the master branch of this repo to be sure..?

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jun 22, 2017

Member

Before we release, have we tested the failing plugins against the master branch of this repo to be sure..?

It looks like this fixes the URL issues, but not the static files frontmatter issue (jekyll/jekyll-sitemap#176)

Member

pathawks commented Jun 22, 2017

Before we release, have we tested the failing plugins against the master branch of this repo to be sure..?

It looks like this fixes the URL issues, but not the static files frontmatter issue (jekyll/jekyll-sitemap#176)

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jun 22, 2017

Member

@pathawks That's fine. @benbalter is working on a fix in #6162.

Member

parkr commented Jun 22, 2017

@pathawks That's fine. @benbalter is working on a fix in #6162.

@outcoldman

This comment has been minimized.

Show comment
Hide comment
@outcoldman

outcoldman Jul 1, 2017

@pathawks any updates on the release? Seems like this plugin is broken jekyll-redirect-from

outcoldman commented Jul 1, 2017

@pathawks any updates on the release? Seems like this plugin is broken jekyll-redirect-from

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jul 1, 2017

Member

@outcoldman Yeah, just waiting on #6162. Sorry.

Member

pathawks commented Jul 1, 2017

@outcoldman Yeah, just waiting on #6162. Sorry.

@outcoldman

This comment has been minimized.

Show comment
Hide comment
@outcoldman

outcoldman Jul 1, 2017

@pathawks seems like it got merged as well! thank you for taking care of it, hope to see 3.5.1 pretty soon 👍

outcoldman commented Jul 1, 2017

@pathawks seems like it got merged as well! thank you for taking care of it, hope to see 3.5.1 pretty soon 👍

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