Skip to content
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

doc: Update documentation on cherry-picking #19619

Open
btracey opened this issue Mar 20, 2017 · 4 comments

Comments

@btracey
Copy link
Contributor

commented Mar 20, 2017

The current documentation on "Reviewing code by others" in the contribution guidelines is for the old Gerrit UI. The new Gerrit UI does not have the same "Download" button, and when one goes to More/Cherry Pick it appears it needs to be cherry-picked onto another branch on Gerrit. I was also unable to reverse engineer the provided git call to apply in a new situation.

It's reasonable to ask users to test if commits resolve issues, but it would be nice to provide clean documentation on how to do so. My recommendations:

  • Making it its own section, possibly above Making a Contribution. Right now, the section is a tiny section at the bottom that is easy to miss.
  • Update the documentation to the new Gerrit UI
  • Some discussion on how to properly apply a patch given an issue, i.e. given an installation from source at current stable, list the correct steps. Checkout branch? Checkout master, then branch, then apply patch?
@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 20, 2017

The download link just moved in the new Gerrit UI. It's now above the list of changed files:

screen shot 2017-03-20 at 9 17 28 am

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 20, 2017

There might be something to do here but I'm apprehensive about adding even more stuff to our contribution guide about using git. We did that at first and we've been trying to undo the damage ever since. It's hard enough to find the Go-specific information in that document.

@btracey

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2017

Thanks for showing me where it is. I missed that.

I understand the apprehension concerning writing a full git tutorial. I do think at least it would be useful to update with the new Gerrit API and moving the section to a more visible location (for those who are asked to apply patches but haven't contributed to Go).

The motivation for the third bullet is that I was unsure how changes would be applied relative to tip. For example, do I need to be on a specific version of tip to correctly apply the patch? If the provided fetch command through "Checkout" will work as desired in most cases, then I don't think there is any need to further update the documentation.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 20, 2017

The motivation for the third bullet is that I was unsure how changes would be applied relative to tip.

Even if we document something (cherry-pick vs checkout), empirically it seems like it'll be wrong about half the time. When I see discussions on bugs or CLs requesting users try a CL, half the time it's "try this exact commit" (git checkout) but half the time it's "try this on along with your local change" (cherry-pick).

And when we're asking people to do that, they're generally developers. If we want users to test something we generally submit it and add some temporary knob (GODEBUG, etc).

@bradfitz bradfitz added this to the Go1.9Maybe milestone Mar 21, 2017

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9Maybe Jul 20, 2017

@rsc rsc modified the milestones: Go1.10, Go1.11 Dec 1, 2017

@ianlancetaylor ianlancetaylor modified the milestones: Go1.11, Unreleased Jun 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.