RFR: Section on PRs [v4]

[Jesper Wilhelmsson]

On Wed, 13 Apr 2022 18:44:07 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> Jesper Wilhelmsson has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Update src/index.md
>>   
>>   Co-authored-by: Daniel Jelinski <djelinski1 at gmail.com>
>
> src/index.md line 1508:
> 
>> 1506: #. **Make sure all relevant groups are included**
>> 1507: 
>> 1508:    The bot will make an attempt to include the groups that need to review your change based on the location of the source code you have changed. There may be aspects of your change that are relevant to other groups as well, and the mapping from source to groups isn't always perfect, so make sure all relevant groups have been included, and add new labels using [`/label`](https://wiki.openjdk.java.net/display/SKARA/Pull+Request+Commands#PullRequestCommands-/label) if needed.
> 
> I would also add something like,
> 
>> If the set of files in the PR has changed, this may affect the groups that need to review the PR. Make sure to adjust the labels accordingly.
> 
> (That leads to the question of how to determine the set of labels given files changed. Is there a way to re-run the bot to refresh the labels? I don't know; for now I'm doing it manually. Fortunately this is a pretty rare occurrence, and I don't think we need to cover it here at the moment.)

Added in the "Updating the PR" section.

> src/index.md line 1515:
> 
>> 1513: 
>> 1514:    At least one reviewer should be knowledgeable in the area being changed. Some areas (e.g. client and hotspot) require two reviewers in most cases, so be sure to read the relevant OpenJDK group pages for advice or ask your sponsor.
>> 1515: 
> 
> I'm not sure whether it belongs in this section or in a different one (possibly earlier) but maybe there needs to be something said about updating the PR in response to review comments. Something like:
> 
>> Updating the PR
>> 
>>    You may need to change the code in response to review comments. To do this, simply commit new changes and push them onto the PR branch. The PR will be updated automatically. Multiple commits to the branch will be "squashed" into a single commit when the PR is eventually integrated.

The squashing part is mentioned earlier, but worth repeating here I think. Added.

> src/index.md line 1518:
> 
>> 1516: #. **Merge the latest changes**
>> 1517: 
>> 1518:    If your PR is out for review for a longer time it's a good habit to pull from the target repository regularly to keep the change up to date. This will make it easier to review the change and it will help you find issues caused by other changes sooner. If there are upstream changes that might affect your change, it's likely a good idea to rerun relevant testing as well. The GHA testing that is done automatically by GitHub should only be seen as a smoke test that finds the most severe problems with your change. It's highly unlikely that it will test your actual change in any greater detail - or even run the code that you have changed in most cases.
> 
> The phrase "pull from the target repository" is a bit vague. I guess it's intended to apply to work on a variety of different projects, not just the mainline JDK. But the common case is for the mainline JDK, so perhaps it should say something like
> 
>> Typically this involves fetching changes from the master branch of the main JDK repo, merging them into your local branch, resolving conflicts if necessary, and then pushing these changes to the PR branch. Pushing additional commits and merges into the PR branch is fine; they will be squashed into a single commit when the PR is integrated. Avoid rebasing changes, and prefer merging instead.

Added.

-------------

PR: https://git.openjdk.java.net/guide/pull/79