Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

[Plugin] Source plugins #4674

Merged
merged 44 commits into from Jul 5, 2016
Merged

[Plugin] Source plugins #4674

merged 44 commits into from Jul 5, 2016

Conversation

asutoshpalai
Copy link
Contributor

Adds source plugin. This is in continuation of #4608.

def source(source, *args, &blk)
options = args.last.is_a?(Hash) ? args.pop.dup : {}
options = normalize_hash(options)
if options && options.key?("type")
Copy link
Member

Choose a reason for hiding this comment

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

options can't be nil here

@@ -38,7 +39,6 @@ module Bundler
autoload :MatchPlatform, "bundler/match_platform"
autoload :Mirror, "bundler/mirror"
autoload :Mirrors, "bundler/mirror"
autoload :Plugin, "bundler/plugin"
Copy link
Member

Choose a reason for hiding this comment

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

why can't this be autoloaded any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/bundler/bundler/pull/4674/files#diff-507c71857bb3b7292c61afb88df1c880L28
This change made starting of bundler without API::Source fail

@asutoshpalai asutoshpalai changed the title [WIP] Plugin (source plugin) [Plugin] Source plugins Jun 22, 2016
@homu
Copy link
Contributor

homu commented Jun 22, 2016

☔ The latest upstream changes (presumably #4700) made this pull request unmergeable. Please resolve the merge conflicts.

@@ -197,7 +197,16 @@ def load_plugin(name)
# done to avoid conflicts
path = index.plugin_path(name)

if insert_index = Bundler.rubygems.load_path_insert_index
Copy link
Member

Choose a reason for hiding this comment

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

we should just be #activateing the spec here, no?

Copy link
Member

Choose a reason for hiding this comment

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

(or using spec.add_self_to_load_path?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to do that. But couldn't find a way to get spec here.

Even if I store the spec's locations, add_self_to_load_path depended on full_gem_path, which itself depended on source.

The alternative to hard coded lib is, we can store the full_required_paths with index.

Copy link
Member

Choose a reason for hiding this comment

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

we have to do something, since it is totally valid for a gem to have a require path that isn't lib

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I am working on extending the serializer to work with array. As soon as I get around that, will fix this.

@allenzhao
Copy link
Contributor

@asutoshpalai I was trying to edit lib/bundler/errors.rb and found that you've used a duplicated status code 23 in L44 which was already taken in L69

@asutoshpalai
Copy link
Contributor Author

@allenzhao Yes, right! Sorry, will fix that with the next push!

@asutoshpalai asutoshpalai force-pushed the plugin branch 2 times, most recently from 69bf74b to 85d2a99 Compare June 28, 2016 14:49
@segiddins
Copy link
Member

@asutoshpalai @indirect is this ready to merge?

@segiddins
Copy link
Member

@homu r+

@homu
Copy link
Contributor

homu commented Jul 5, 2016

📌 Commit 57e8172 has been approved by segiddins

homu added a commit that referenced this pull request Jul 5, 2016
[Plugin] Source plugins

Adds source plugin. This is in continuation of #4608.
@homu
Copy link
Contributor

homu commented Jul 5, 2016

⌛ Testing commit 57e8172 with merge cc4df62...

@homu
Copy link
Contributor

homu commented Jul 5, 2016

☀️ Test successful - status

@homu homu merged commit 57e8172 into rubygems:master Jul 5, 2016
@segiddins segiddins mentioned this pull request Aug 11, 2016
@coilysiren coilysiren modified the milestone: Release Archive Sep 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants