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

Improve the Download message in PR description #1053

Closed

Conversation

magicus
Copy link
Member

@magicus magicus commented Mar 12, 2021

Both @simonis and I have struggled of finding the proper way to update a checked-out PR. Let's update the help text with Volker's findings of the best way to do this!


Progress

  • Change must not contain extraneous whitespace
  • Change must be properly reviewed

Reviewers

Download

$ git fetch https://git.openjdk.java.net/skara pull/1053/head:pull/1053
$ git checkout pull/1053

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 12, 2021

👋 Welcome back ihse! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot added the rfr label Mar 12, 2021
@mlbridge
Copy link

mlbridge bot commented Mar 12, 2021

Webrevs

@edvbld
Copy link
Member

edvbld commented Mar 12, 2021

You mean when you are updating your local copy of someone else's pull request? Because otherwise you just pull from your personal fork, right?

@magicus
Copy link
Member Author

magicus commented Mar 12, 2021

If you follow the instructions in the Download section to get a local copy of the PR, this works just fine. However, if you want to update your local copy when the PR has changed with new code, this turned out to be a non-trivial exercise in gitology for both me and Volker (and many more, I presume). I think this is understood from the context of the "Download" part.

@magicus
Copy link
Member Author

magicus commented Mar 12, 2021

@edvbld
Copy link
Member

edvbld commented Mar 16, 2021

Ok, I see what you want to do. My below instructions are not appropriate to add to the pull request body (due to the complexity of the commands), but the most ergonomic way to set this up is to just add a remote and add an additional fetch command:

  1. $ git remote add upstream https://git.openjdk.java.net/skara
  2. $ git config --add remote.upstream.fetch '+refs/pull/*/head:refs/remotes/upstream/pull/*

Then whenever you want to checkout a pull request locally you can just run

$ git fetch upstream # in case you haven't fetched in a while
$ git checkout -b pull/1053

The above will ensure that the local branch pull/1053 tracks refs/remotes/upstream/pull/1053 which in turn is updated from refs/pull/1053/head whenever you run git fetch upstream 😄 Again, a little bit too long to add this information to pull request body, so I think we just go with your changes in this PR. Could potentially be something for an FAQ or the developer's guide?

edvbld
edvbld approved these changes Mar 16, 2021
Copy link
Member

@edvbld edvbld left a comment

Looks good

@openjdk
Copy link

openjdk bot commented Mar 16, 2021

@magicus This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

🔍 One or more changes in this pull request modifies files in areas of the source code that often require two reviewers. Please consider if this is the case for this pull request, and if so, await a second reviewer to approve this pull request before you integrate it.

After integration, the commit message for the final commit will be:

Improve the Download message in PR description

Reviewed-by: ehelin

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 5 new commits pushed to the master branch:

  • 902698a: vcs: add ReadOnlyRepository.commitMetadataFor
  • b39d50b: Add missing copyright headers
  • 0b326a6: pr: add /tag commit command
  • d93932e: vcs: add support for pushing single tag
  • 263bdaf: forge: add webUrl(Tag tag) method

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Mar 16, 2021
@magicus
Copy link
Member Author

magicus commented Mar 16, 2021

/integrate

@openjdk openjdk bot closed this Mar 16, 2021
@openjdk openjdk bot added integrated and removed ready rfr labels Mar 16, 2021
@openjdk
Copy link

openjdk bot commented Mar 16, 2021

@magicus Since your change was applied there have been 9 commits pushed to the master branch:

  • dc052db: pr: use bare repo for CommitCommentsWorkItem
  • 2ca0412: Add support for using ssh cloning with GitLab
  • 79e2056: vcs: make RepositoryTests deterministic
  • 763af0c: bot: don't run watchdogTrigger test on Windows
  • 902698a: vcs: add ReadOnlyRepository.commitMetadataFor
  • b39d50b: Add missing copyright headers
  • 0b326a6: pr: add /tag commit command
  • d93932e: vcs: add support for pushing single tag
  • 263bdaf: forge: add webUrl(Tag tag) method

Your commit was automatically rebased without conflicts.

Pushed as commit a6a7efa.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@simonis
Copy link
Member

simonis commented Mar 16, 2021

Thanks @magicus for updating the download message and thanks @edvbld for shedding some light on how to properly define an upstream for pull requests :) If I understand you correct, with that setup a git fetch upstream will fetch all pull requests from upstream. Is that correct?

@edvbld
Copy link
Member

edvbld commented Mar 17, 2021

@simonis yep, that is correct. As usual when you fetch refs from a remote they will only becomes refs under refs/remote/<remote-name>/<branch-name>, so you won't see all the refs when running e.g. git branch (you would have to run git branch -a).

@simonis
Copy link
Member

simonis commented Mar 17, 2021

@edvbld thanks for the confirmation. I'll definitely put that line into my .git/config (if only to not forget the syntax :)

@magicus magicus deleted the improve-download-instructions branch Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants