Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Fix missing branch in dep ensure -add: introduce hoist from lock #994

Merged
merged 3 commits into from
Aug 12, 2017

Conversation

sdboyer
Copy link
Member

@sdboyer sdboyer commented Aug 11, 2017

What does this do / why do we need it?

This completes a major bit of #489 that was missing - converting whatever's in the lock to a constraint that we can append into Gopkg.toml when no version constraint was given on the CLI.

What should your reviewer look out for in this PR?

This was easier than i thought it would be. Have i missed something?

@sdboyer sdboyer requested a review from ibrasho as a code owner August 11, 2017 23:44
@sdboyer sdboyer changed the title Add hoist from lock Add hoist from lock to missing branch of dep ensure -add Aug 11, 2017
@sdboyer sdboyer changed the title Add hoist from lock to missing branch of dep ensure -add Fix missing branch in dep ensure -add: introduce hoist from lock Aug 12, 2017
@ibrasho
Copy link
Collaborator

ibrasho commented Aug 12, 2017

LGTM from a quick glance.

QQ: Is there any possibility that solution.Projects() won't contain the project of an -add argument?

@sdboyer
Copy link
Member Author

sdboyer commented Aug 12, 2017

Is there any possibility that solution.Projects() won't contain the project of an -add argument?

not if we successfully found a solution. i was thinking about this while coding it, though. i'll add a panic for that case - better to be explicit.

@sdboyer
Copy link
Member Author

sdboyer commented Aug 12, 2017

ok, goin in

@ibrasho
Copy link
Collaborator

ibrasho commented Aug 12, 2017

I think we still need to resolve manifest verification just in case we create an invalid manifest somehow.

@sdboyer
Copy link
Member Author

sdboyer commented Aug 12, 2017

@ibrasho oh, for sure - just, separate issue.

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

Successfully merging this pull request may close these issues.

3 participants