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

add `plugins` config key as replacement for `gems` #5130

Merged
merged 12 commits into from Apr 17, 2017

Conversation

Projects
None yet
@Crunch09
Member

Crunch09 commented Jul 24, 2016

gems won't be deprecated. This fixes #4509 .

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 24, 2016

Member

Just two quick comments.

@benbalter, anything further here?

Member

parkr commented Jul 24, 2016

Just two quick comments.

@benbalter, anything further here?

@parkr parkr added the enhancement label Jul 24, 2016

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter Jul 25, 2016

Contributor

won't be deprecated.

. This is exactly what I was proposing. Nice work. A quick win for usability.

Contributor

benbalter commented Jul 25, 2016

won't be deprecated.

. This is exactly what I was proposing. Nice work. A quick win for usability.

@parkr parkr modified the milestones: 3.2, 3.3 Jul 25, 2016

@Crunch09

This comment has been minimized.

Show comment
Hide comment
@Crunch09

Crunch09 Jul 26, 2016

Member

@parkr Thanks for the review! I tried to address your comments and also changed Site#gems and Site#plugins to Site#plugins and Site#plugins_dir to match their Configuration counterparts.
I didn't change the display of a deprecation warning so far because of @benbalter 's comment.

Member

Crunch09 commented Jul 26, 2016

@parkr Thanks for the review! I tried to address your comments and also changed Site#gems and Site#plugins to Site#plugins and Site#plugins_dir to match their Configuration counterparts.
I didn't change the display of a deprecation warning so far because of @benbalter 's comment.

Show outdated Hide outdated lib/jekyll/site.rb
Show outdated Hide outdated lib/jekyll/site.rb
Show outdated Hide outdated lib/jekyll/site.rb
@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 27, 2016

Contributor

This is great, just a few comments!

Contributor

envygeeks commented Jul 27, 2016

This is great, just a few comments!

@Crunch09

This comment has been minimized.

Show comment
Hide comment
@Crunch09

Crunch09 Jul 27, 2016

Member

Changed it back so Site#plugins and Site#gems doesn't change their behavior.

Also, Config#gems IS now deprecated.

Member

Crunch09 commented Jul 27, 2016

Changed it back so Site#plugins and Site#gems doesn't change their behavior.

Also, Config#gems IS now deprecated.

@parkr parkr modified the milestones: 3.3, 3.4 Sep 30, 2016

@parkr parkr modified the milestones: 3.4, 3.5 Jan 16, 2017

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Mar 27, 2017

Member

@Crunch09 sorry for the stale, could you please fix the conflicts before @jekyll/stability can review this PR?

Member

DirtyF commented Mar 27, 2017

@Crunch09 sorry for the stale, could you please fix the conflicts before @jekyll/stability can review this PR?

@Crunch09

This comment has been minimized.

Show comment
Hide comment
@Crunch09

Crunch09 Mar 27, 2017

Member

@DirtyF Thank you for taking a look at this! I just rebased against master and hope the tests still run 😄 If i should squash the commits as well just let me know

Member

Crunch09 commented Mar 27, 2017

@DirtyF Thank you for taking a look at this! I just rebased against master and hope the tests still run 😄 If i should squash the commits as well just let me know

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Jul 1, 2017

Member

that's the entire point

okay, can it be toned down to an info level message at least?

Member

ashmaroli commented Jul 1, 2017

that's the entire point

okay, can it be toned down to an info level message at least?

@Crunch09

This comment has been minimized.

Show comment
Hide comment
@Crunch09

Crunch09 Jul 1, 2017

Member

I agree with @envygeeks that we need to keep the deprecation message so we can remove the old functionality in Jekyll 4.
I'm afraid info is too easy to be overseen. Maybe warn instead of error would be the right choice for deprecation messages instead?

Member

Crunch09 commented Jul 1, 2017

I agree with @envygeeks that we need to keep the deprecation message so we can remove the old functionality in Jekyll 4.
I'm afraid info is too easy to be overseen. Maybe warn instead of error would be the right choice for deprecation messages instead?

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 1, 2017

Contributor

WARN is fitting, that's how I was informed of changes in Ruby 2.4 going into the next major version and only on the CI which explicitly enables warnings.

Contributor

envygeeks commented Jul 1, 2017

WARN is fitting, that's how I was informed of changes in Ruby 2.4 going into the next major version and only on the CI which explicitly enables warnings.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Jul 1, 2017

Member

so we can remove the old functionality in Jekyll 4

Yes, that's true.. But then, we're nowhere near even starting development on a v4, right? (Or are we?) So, i thought we'll display that message when we're at around v3.8.0..

Member

ashmaroli commented Jul 1, 2017

so we can remove the old functionality in Jekyll 4

Yes, that's true.. But then, we're nowhere near even starting development on a v4, right? (Or are we?) So, i thought we'll display that message when we're at around v3.8.0..

@Crunch09

This comment has been minimized.

Show comment
Hide comment
@Crunch09

Crunch09 Jul 1, 2017

Member

I don't know what the release plan will be until Jekyll 4 but removing the deprecation now and bringing it back later would be very confusing i think.

Member

Crunch09 commented Jul 1, 2017

I don't know what the release plan will be until Jekyll 4 but removing the deprecation now and bringing it back later would be very confusing i think.

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jul 1, 2017

Member

So, i thought we'll display that message when we're at around v3.8.0..

You are planning to release 3.8.0?? News to me.

Many users have experienced pain when something unexpectedly changes and their site won't build all of a sudden. This is why we offer deprecation messages: to give them plenty of time to address these things.

We know we will change this when we have an opportunity; why would we not communicate that to users? Why would we want to give users less warning to update their site?

Member

pathawks commented Jul 1, 2017

So, i thought we'll display that message when we're at around v3.8.0..

You are planning to release 3.8.0?? News to me.

Many users have experienced pain when something unexpectedly changes and their site won't build all of a sudden. This is why we offer deprecation messages: to give them plenty of time to address these things.

We know we will change this when we have an opportunity; why would we not communicate that to users? Why would we want to give users less warning to update their site?

@pathawks

This comment has been minimized.

Show comment
Hide comment
@pathawks

pathawks Jul 1, 2017

Member

Its kinda annoying seeing that message

You could always make it go away by, you know, following its advice.

Member

pathawks commented Jul 1, 2017

Its kinda annoying seeing that message

You could always make it go away by, you know, following its advice.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 1, 2017

Member

I don't see a path for Jekyll 4 yet, as we don't have anything absolutely crucial that we can't do in a backwards-compatible way. I would like to keep our tradition of shipping backwards-compatible changes instead of breaking things.

This deprecation is annoying for users who want to use this version but are shipping out to GitHub Pages where the plugins key is not yet supported. Pages will upgrade in due time; for now, I recommend downgrading to Jekyll v3.4.5 for all Pages users' day-to-day needs.

Member

parkr commented Jul 1, 2017

I don't see a path for Jekyll 4 yet, as we don't have anything absolutely crucial that we can't do in a backwards-compatible way. I would like to keep our tradition of shipping backwards-compatible changes instead of breaking things.

This deprecation is annoying for users who want to use this version but are shipping out to GitHub Pages where the plugins key is not yet supported. Pages will upgrade in due time; for now, I recommend downgrading to Jekyll v3.4.5 for all Pages users' day-to-day needs.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 1, 2017

Member

If the message isn't clear or isn't clearly actionable, then I'm open to a PR that rewords the message for clarity.

Member

parkr commented Jul 1, 2017

If the message isn't clear or isn't clearly actionable, then I'm open to a PR that rewords the message for clarity.

@ashmaroli

This comment has been minimized.

Show comment
Hide comment
@ashmaroli

ashmaroli Jul 1, 2017

Member

Looks like I stirred up a hornet's nest of sorts..
I surrender 🏳️

🔥

Member

ashmaroli commented Jul 1, 2017

Looks like I stirred up a hornet's nest of sorts..
I surrender 🏳️

🔥

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 1, 2017

Member

All good! Just wanted to be clear here. 😃

Member

parkr commented Jul 1, 2017

All good! Just wanted to be clear here. 😃

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Jul 1, 2017

Contributor

I would like to keep our tradition of shipping backwards-compatible changes instead of breaking things.

Might as well throw that out the window @parkr. Jekyll 3.5 was a total disaster for some plugins because of breaking core changes. Namely in context of Liquid 4, and I'm sure there are hundreds of plugins that haven't even noticed yet, but we felt that pain on Jekyll-Assets when we had to wrap around the breaking class to support everything BEFORE 3.5 and everything AFTER 3.5. You forced us to make a split on logic based on a breaking change already 😉

Contributor

envygeeks commented Jul 1, 2017

I would like to keep our tradition of shipping backwards-compatible changes instead of breaking things.

Might as well throw that out the window @parkr. Jekyll 3.5 was a total disaster for some plugins because of breaking core changes. Namely in context of Liquid 4, and I'm sure there are hundreds of plugins that haven't even noticed yet, but we felt that pain on Jekyll-Assets when we had to wrap around the breaking class to support everything BEFORE 3.5 and everything AFTER 3.5. You forced us to make a split on logic based on a breaking change already 😉

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Jul 1, 2017

Member

I'm sorry to hear it was a total disaster. Most plugins I use still worked great. I know jekyll-assets uses some APIs that we don't consider public, but the principle is the same. I'm sorry for shipping breaking changes. We'll try to do better next time. When we upgrade in a minor release, we'll make sure it's simply adding support for the new version rather than removing support for the old version as we did with Liquid.

We want to upgrade to Rouge 2. I'll be sure to ship an rc first and give two weeks before we ship the finished product so you we can test things out. I'll open an issue on jekyll-assets when the rc for 3.6 ships.

Member

parkr commented Jul 1, 2017

I'm sorry to hear it was a total disaster. Most plugins I use still worked great. I know jekyll-assets uses some APIs that we don't consider public, but the principle is the same. I'm sorry for shipping breaking changes. We'll try to do better next time. When we upgrade in a minor release, we'll make sure it's simply adding support for the new version rather than removing support for the old version as we did with Liquid.

We want to upgrade to Rouge 2. I'll be sure to ship an rc first and give two weeks before we ship the finished product so you we can test things out. I'll open an issue on jekyll-assets when the rc for 3.6 ships.

@alzeih alzeih referenced this pull request Jul 28, 2017

Closed

Change RSS references #129

@ryzokuken ryzokuken referenced this pull request Aug 8, 2017

Merged

Rename gems key to plugins #23

mmistakes added a commit to mmistakes/minimal-mistakes that referenced this pull request Aug 8, 2017

Rename gems key to `plugins`
- `gems` key in `_config.yml`  was deprecated in Jekyll 3.5 and changed to `plugins`

ref: jekyll/jekyll#5130

lizconlan added a commit to mysociety/alaveteli that referenced this pull request Aug 15, 2017

Rename gems config to plugins
'gems' has been deprecated in favour of plugins
jekyll/jekyll#5130

lizconlan added a commit to mysociety/alaveteli that referenced this pull request Aug 15, 2017

Rename gems config to plugins
'gems' has been deprecated in favour of 'plugins'
jekyll/jekyll#5130

BoWuGit pushed a commit to BoWuGit/bowugit.github.io that referenced this pull request Aug 17, 2017

Rename gems key to `plugins`
- `gems` key in `_config.yml`  was deprecated in Jekyll 3.5 and changed to `plugins`

ref: jekyll/jekyll#5130

lizconlan added a commit to mysociety/alaveteli that referenced this pull request Aug 22, 2017

Rename gems config to plugins
'gems' has been deprecated in favour of 'plugins'
jekyll/jekyll#5130

amweiss added a commit to amweiss/amweiss.github.io that referenced this pull request Aug 22, 2017

Rename gems key to `plugins`
- `gems` key in `_config.yml`  was deprecated in Jekyll 3.5 and changed to `plugins`

ref: jekyll/jekyll#5130

mmistakes added a commit to mmistakes/minimal-mistakes that referenced this pull request Sep 11, 2017

Rename `gems` key to `plugins` (#1239)
`gems` key in `_config.yml` was deprecated in Jekyll 3.5 and changed to `plugins`.

ref: jekyll/jekyll#5130
@deanilvincent

This comment has been minimized.

Show comment
Hide comment
@deanilvincent

deanilvincent Sep 12, 2017

screen shot 2017-09-12 at 9 48 02 pm

I got this error while trying to run my github page website locally. Any help?

deanilvincent commented Sep 12, 2017

screen shot 2017-09-12 at 9 48 02 pm

I got this error while trying to run my github page website locally. Any help?

@sourcejedi

This comment has been minimized.

Show comment
Hide comment
@sourcejedi

sourcejedi Sep 12, 2017

@deanilvincent The Deprecation message is not the error. The only error is the last line. There is no reason here to believe it is related to this PR.

sourcejedi commented Sep 12, 2017

@deanilvincent The Deprecation message is not the error. The only error is the last line. There is no reason here to believe it is related to this PR.

@deanilvincent

This comment has been minimized.

Show comment
Hide comment
@deanilvincent

deanilvincent commented Sep 13, 2017

Thank you @sourcejedi :)

@jekyll jekyll locked and limited conversation to collaborators Sep 13, 2017

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