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 `gems` for better plugin management #1557

Merged
merged 3 commits into from Oct 23, 2013

Conversation

Projects
None yet
5 participants
@parkr
Member

parkr commented Sep 16, 2013

Some plugins are distributed as gems. Why not give better support for them by requiring them explicitly? (in unsafe mode)?

Fulfills #1417.

  • Implement
  • Write test gem
  • Add documentation

/cc @mattr- @benbalter

@mattr-

This comment has been minimized.

Member

mattr- commented Sep 17, 2013

The changes to the comment should probably be moved out into a separate commit, but I'm ok with it otherwise.

@benbalter

This comment has been minimized.

Contributor

benbalter commented Oct 4, 2013

To clarify, this adds a gems option to the config file?

I can't include a random, unrelated gem using that method, can I? e.g., what happens if I put gem: 'rails'.

I'd make the intent specific, and would rather name it something like plugins if possible.

Also, looks like you've got some permalink related stuff in this pull request.

@parkr

This comment has been minimized.

Member

parkr commented Oct 4, 2013

@benbalter Great points? We have a plugins setting already – it sets the plugins directory location. :)

You can totally include a random, unrelated gem, but that makes no sense, so hopefully no one would do it!

Yeah, I guess I should do the permalink stuff elsewhere. I just didn't want to forget it.

@troyswanson

View changes

site/docs/upgrading.md Outdated
<p markdown="1">
Starting with Jekyll v1.1.0, `relative_permalinks` will default to `false`,
Starting with Jekyll v2.0, permalinks` will default to `false`,

This comment has been minimized.

@troyswanson

troyswanson Oct 4, 2013

Member

I think you may have missed a backtick here

@parkr

This comment has been minimized.

Member

parkr commented Oct 4, 2013

Once we decide it's a good idea, I'll write docs. :)

@parkr

This comment has been minimized.

Member

parkr commented Oct 15, 2013

Thoughts?

@mattr-

This comment has been minimized.

Member

mattr- commented Oct 16, 2013

Does this handle safe mode correctly? Can't quite tell.

@parkr

This comment has been minimized.

Member

parkr commented Oct 16, 2013

Yeah, it's within the same block as the plugin require's.

@parkr

This comment has been minimized.

Member

parkr commented Oct 16, 2013

I'd love to see this in v1.3.

@mattr-

This comment has been minimized.

Member

mattr- commented Oct 18, 2013

The implementation looks fine.

I'm wondering thought why this just can't work by using Bundler. Stick the plugin gems in a Gemfile and only run through the Bundler.require stuff if in safe mode.

@parkr

This comment has been minimized.

Member

parkr commented Oct 18, 2013

Some systems can't use bundler and some people don't use bundler? I'm not sure I've ever heard of Bundler.require before, and most plugin managers/users likely won't have either. I was thinking this would be most similar to current behaviour and we can offer custom error messages for jekyll-specific requiring.

@mattr-

This comment has been minimized.

Member

mattr- commented Oct 18, 2013

Bundler.require is a method that handles requiring all the gems in your Gemfile if I understand Bundler's API correctly. The more that I think about it though, the more I like your simpler solution better.

@parkr parkr referenced this pull request Oct 18, 2013

Closed

Plugin Ecosystem #1272

mattr- added a commit that referenced this pull request Oct 23, 2013

Merge pull request #1557 from mojombo/require-gems-plugins
Add `gems` for better plugin management

@mattr- mattr- merged commit 23ad7fa into master Oct 23, 2013

1 check passed

default The Travis CI build passed
Details

@mattr- mattr- deleted the require-gems-plugins branch Oct 23, 2013

mattr- added a commit that referenced this pull request Oct 23, 2013

@parkr

This comment has been minimized.

Member

parkr commented Oct 23, 2013

YAY 🎉 😄 😉 👍 💯 🆒

@huricanne

This comment has been minimized.

huricanne commented on 6945996 Oct 26, 2013

Is falk that it see who go on

<p markdown="1">
Starting with Jekyll v1.1.0, `relative_permalinks` will default to `false`,
Starting with Jekyll v2.0, `relative_permalinks` will default to `false`,

This comment has been minimized.

@benbalter

benbalter Nov 2, 2013

Contributor

Too late now, but FYI this stuff snuck in.

This comment has been minimized.

@parkr

parkr Nov 2, 2013

Member

We need to do this anyway

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