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

FAQ entry for issue #856, (anyway I *think* this is how it works) #863

Merged
merged 11 commits into from
Aug 11, 2017

Conversation

djosephsen
Copy link
Contributor

@djosephsen djosephsen commented Jul 20, 2017

What does this do / why do we need it?

Just some words for the faq / Because you asked for them in #856

What should your reviewer look out for in this PR?

no sharp edges here.

Do you need help or clarification on anything?

Nope 👍

Which issue(s) does this PR fix?

#865

@djosephsen djosephsen requested a review from sdboyer as a code owner July 20, 2017 16:44
@djosephsen djosephsen changed the title FAQ entry for issue #865, (anyway I *think* this is how it works) FAQ entry for issue #856, (anyway I *think* this is how it works) Jul 20, 2017
FAQ.md Outdated

## One or more of my dependencies doesn't tag its releases. What should I do?

Simply add a constrainit to your manifest that specifies `"branch": "master"`
Copy link
Contributor

Choose a reason for hiding this comment

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

spelling: constraint

FAQ.md Outdated
## One or more of my dependencies doesn't tag its releases. What should I do?

Simply add a constrainit to your manifest that specifies `"branch": "master"`
for the dependency. Dep will deduce the current revision number of your
Copy link
Contributor

Choose a reason for hiding this comment

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

use dep not Dep, I think.

Copy link
Collaborator

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

Just two small changes. Thank you for tackling this so quickly!

FAQ.md Outdated
@@ -34,6 +34,7 @@ Summarize the question and quote the reply, linking back to the original comment
* [What semver version should I use?](#what-semver-version-should-i-use)
* [Is it OK to make backwards-incompatible changes now?](#is-it-ok-to-make-backwards-incompatible-changes-now)
* [My dependers don't use `dep` yet. What should I do?](#my-dependers-dont-use-dep-yet-what-should-i-do)
* [One or more of my dependencies doesn't tag its releases. What should I do?](#one-or-more-of-my-dependencies-dont-tag-their-releases-what-should-i-do)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's simplify the question to "How do I configure a dependency that doesn't tag its releases?"

FAQ.md Outdated

## One or more of my dependencies doesn't tag its releases. What should I do?

Simply add a constraint to your manifest that specifies `"branch": "master"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove "simply", changing it to "Add a constraint...". I don't want anyone feeling bad about themselves if this doesn't feel simple! 😀

@carolynvs
Copy link
Collaborator

Don't worry about the PR status checks by the way! Github has a bug at the moment which is making them fail when the PR is just fine.

Dave Josephsen and others added 3 commits July 20, 2017 12:56
FAQ.md Outdated

## How do I configure a dependency that doesn't tag its releases?

Add a constraint to your manifest that specifies `"branch": "master"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ooops, I missed this the first time around but it should be branch = "master" in order to be proper TOML.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

blerg sorry.

FAQ.md Outdated
@@ -34,8 +34,7 @@ Summarize the question and quote the reply, linking back to the original comment
* [What semver version should I use?](#what-semver-version-should-i-use)
* [Is it OK to make backwards-incompatible changes now?](#is-it-ok-to-make-backwards-incompatible-changes-now)
* [My dependers don't use `dep` yet. What should I do?](#my-dependers-dont-use-dep-yet-what-should-i-do)

___
* [How do I configure a dependency that doesn't tag its releases?](#how-do-I-configure-a-dependency-that-doesnt-tag-its-releases)
Copy link
Collaborator

Choose a reason for hiding this comment

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

One more change, I swear! Thank you for sticking with me. 😁

The link isn't scrolling down the page to the right section because of the upper-case "I" in the url. It should be this #how-do-i-configure-a-dependency-that-doesnt-tag-its-releases.

I tried to "help" before by adding a commit to your PR but I think that made it harder... Sorry about that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

np really. Making epics of what should be a simple change is my superpower

@djosephsen
Copy link
Contributor Author

omg. I actually can't believe I just did that standby..

@googlebot
Copy link
Collaborator

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@djosephsen
Copy link
Contributor Author

Not sure why it thinks I didn't sign the cla (I did)

Copy link
Collaborator

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

@sdboyer Would you please do a squash and merge for us? :bowtie:

@carolynvs
Copy link
Collaborator

Don't worry about the CLA, there's a bug at the moment which is causing it to fail. Thanks again for the PR! 💖

Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

some quick changes, first...

FAQ.md Outdated

Add a constraint to your manifest that specifies `branch: "master"`
for the dependency. `dep` will deduce the current revision number of your
dependencies master branch, and place it the lock-file (`Gopkg.lock`) for you.
Copy link
Member

Choose a reason for hiding this comment

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

s/dependencies/dependency's/

FAQ.md Outdated
## How do I configure a dependency that doesn't tag its releases?

Add a constraint to your manifest that specifies `branch: "master"`
for the dependency. `dep` will deduce the current revision number of your
Copy link
Member

Choose a reason for hiding this comment

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

s/dep/dep ensure/ - just to make it clear what phase (e.g., not before dep init) in which this guidance applies.

Copy link
Member

Choose a reason for hiding this comment

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

Also, s/revision number/revision/

FAQ.md Outdated
Add a constraint to your manifest that specifies `branch: "master"`
for the dependency. `dep` will deduce the current revision number of your
dependencies master branch, and place it the lock-file (`Gopkg.lock`) for you.
Anyone who clones your project will, therefore, wind up with same version of the dependency,
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is quite what we want to be saying here. Committing vendor is what would guarantee this at clone time; having the rev in your Gopkg.lock only guarantees that vendor will be populated with it by dep ensure (assuming no other changes).

Just the last sentence, referring the reader to the other question, is probably sufficient.

@sdboyer sdboyer added the docs label Jul 21, 2017
Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

tiny nit, then needs a rebase, then we're good 😄

FAQ.md Outdated
## How do I configure a dependency that doesn't tag its releases?

Add a constraint to your manifest that specifies `branch: "master"`
for the dependency. `dep ensure` will deduce the current revision of your
Copy link
Member

Choose a reason for hiding this comment

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

eh, one last tiny nit - we often use "deduce" to mean a specific thing in the dep/gps world. there's nothing wrong with this usage here, but if you did s/determine/deduce/, it would help avoid any possible inferred relationship forming in the mind of the reader between the deduction subsystem (if they know about it) and figuring out the underlying rev of a branch.

Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

i have some small tweaks to make, but i'll do those directly rather than having this languish further. sorry for the slowness!

@sdboyer
Copy link
Member

sdboyer commented Aug 11, 2017

oh wait, conflicts. ok, i'll fix directly, then push 😄

@sdboyer sdboyer merged commit f53c745 into golang:master Aug 11, 2017
@sdboyer
Copy link
Member

sdboyer commented Aug 11, 2017

@djosephsen thank you!

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.

5 participants