Skip to content

Upgraded to Go 1.9 and dep#94

Closed
febbraro wants to merge 4 commits intodevelopfrom
feature/dep
Closed

Upgraded to Go 1.9 and dep#94
febbraro wants to merge 4 commits intodevelopfrom
feature/dep

Conversation

@febbraro
Copy link
Member

Move away from Godep to dep (thanks to @sdboyer)

This is for #68

@febbraro febbraro requested review from grayside and tekante October 20, 2017 22:09
Copy link
Contributor

@grayside grayside left a comment

Choose a reason for hiding this comment

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

I don't see travis updates. There are hidden godep steps there, as well as an env var related to vendoring.

packages = ["."]
revision = "cd8b52f8269e0feb286dfeef29f8fe4d5b397e0b"

[solve-meta]
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure where it belongs, but depending readme recommends ci should use a fixed version.

Copy link
Contributor

Choose a reason for hiding this comment

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

dep readme, not depending

@febbraro
Copy link
Member Author

@grayside Updated based on feedback. Multiple dependencies did not have explicit versions, so I pinned to a revision

@febbraro febbraro mentioned this pull request Oct 21, 2017
@sdboyer
Copy link

sdboyer commented Oct 24, 2017

hola friends! couple quick notes on this:

  1. you'll probably have an easier time if you use release versions of dep instead of tip.
  2. if there's ever a possibility that something else will depend on rig, you'll likely want to place the Gopkg.{toml,lock},vendor at the repository root.
  3. in general, you needn't specify a revision in Gopkg.toml - instead, trust Gopkg.lock to pin things appropriately. if there's no tags to consume, prefer a branch. then just make sure to always use dep ensure -update <deps...> instead of dep ensure -update (globally), and those pinned versions won't change.

@grayside
Copy link
Contributor

Thanks @sdboyer ! (1) sounds good, (2) we decided to do in a follow-up if only for standards & practices, (3) Aha, that makes sense from npm/composer/etc.

@febbraro
Copy link
Member Author

Thanks @sdboyer! Appreciate the feedback

@grayside grayside added this to the v2.0 milestone Oct 25, 2017
@febbraro
Copy link
Member Author

Moving all this work to #95

@febbraro febbraro closed this Oct 25, 2017
@febbraro febbraro deleted the feature/dep branch October 25, 2017 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants