RFR: Section on sponsoring [v9]

Fri Aug 11 02:10:28 UTC 2023

On Thu, 3 Aug 2023 09:42:23 GMT, Jesper Wilhelmsson <jwilhelm at openjdk.org> wrote:

>> Copied and updated the text from the old sponsoring page: https://openjdk.org/sponsor/
>> The purpose is to have the sponsor link on the OpenJDK page lead here instead.
>> 
>> Also updated the contributing section with some parts taken from the old contributing page: https://openjdk.org/contribute/
>> Again, the intention is to change the link on the OpenJDK page to lead here.
>
> Jesper Wilhelmsson has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Be mindful

Hi Jesper,

A few comments below - just my opinion so take them, or leave them, as you will. A few typos too.

Thanks.

src/guide/reviewing-and-sponsoring-a-change.md line 11:

> 9: Two of the most important contributions you can make in the OpenJDK community are to review and sponsor changes for other developers. All changes needs to be reviewed. Many changes even need to be reviewed by more than one person. This means that in order to enable a fast-pased development experience, all developers need to review more changes then they produce themself.
> 10: 
> 11: If you are new to an area, reviewing changes is also a great way to learn the code and see what general styles and types of changes that are relevant in the area. Be mindful though, if you don't know the area well you should state this in your review comments. As a **R**eviewer you have a responsibility to make sure changes are sound and goes in line with the general direction of the area. If you, as a **R**eviever, review a change in an area that you don't know well you probably shouldn't be the one to approve the change.

s/changes that/that/
s/goes in line/go in line/ or perhaps better just "and align with ..."

src/guide/reviewing-and-sponsoring-a-change.md line 13:

> 11: If you are new to an area, reviewing changes is also a great way to learn the code and see what general styles and types of changes that are relevant in the area. Be mindful though, if you don't know the area well you should state this in your review comments. As a **R**eviewer you have a responsibility to make sure changes are sound and goes in line with the general direction of the area. If you, as a **R**eviever, review a change in an area that you don't know well you probably shouldn't be the one to approve the change.
> 12: 
> 13: New developers in the OpenJDK community don't have the permissions needed to push changes to the repositories. This is a feature that ensures that all developers get familiar with the code, processes, and community before being allowed to actually make changes. To get the first changes in, the new Contributor needs the help of a Sponsor. The Sponsor's role is to offer constructive advice and eventually push the sponsored contribution into the repository.

s/get the first/get their first/
s/eventually push/eventually integrate/ (the developer does the git push)

src/guide/reviewing-and-sponsoring-a-change.md line 15:

> 13: New developers in the OpenJDK community don't have the permissions needed to push changes to the repositories. This is a feature that ensures that all developers get familiar with the code, processes, and community before being allowed to actually make changes. To get the first changes in, the new Contributor needs the help of a Sponsor. The Sponsor's role is to offer constructive advice and eventually push the sponsored contribution into the repository.
> 14: 
> 15: Sponsoring new contributions is an important activity - it's how the engineering culture of a project gets passed on from the core group to new Contributors, from one generation to the next. It should be fun, so please celebrate the contributions you've sponsored by mentioning them on your blog or similar. Congratulate other Sponsors on their work. Take pride in the value you provide to Contributors. Their success reflects well on you.

This paragraph is a bit over-the-top for me. I would personally drop the "It should fun ..." and "Congratulate ..." sentences.

src/guide/reviewing-and-sponsoring-a-change.md line 17:

> 15: Sponsoring new contributions is an important activity - it's how the engineering culture of a project gets passed on from the core group to new Contributors, from one generation to the next. It should be fun, so please celebrate the contributions you've sponsored by mentioning them on your blog or similar. Congratulate other Sponsors on their work. Take pride in the value you provide to Contributors. Their success reflects well on you.
> 16: 
> 17: As a Sponsor, Contributors will look up to you for guidance to get their contributions into the project - your actions will determine whether Contributors will feel welcome and want to engage further with the project beyond their initial attempt, or not. Let's not lose enthusiastic, engaged and technically competent Contributors due to a lack of communication. If there is a request in your area of expertise and you can't address it, at least acknowledge receipt of the request and provide an estimate for when you'll be able to give it your attention. A frank explanation of your time constraints or commitments will be appreciated and respected.

This, to me, makes being a Sponsor sound like an active role i.e. you sit and watch all day for things that your might Sponsor; or there is a notion of a designated sponsor for an area who may not have time for something at the moment. But normally it is not like that. If a change has been socialized ahead of time then a sponsor tends to emerge from the participants in the PR. If a change has not been socialized and a PR just appears then it may well just sit there because it isn't on anyone's radar - not even to say "I'm too busy for this" because they are too busy to pay it any attention in the first place.

src/guide/reviewing-and-sponsoring-a-change.md line 29:

> 27: ### Conversation
> 28: 
> 29: The Conversation section is where all discussion around the PR happens. Note that this section is intended for technical discussion around the actual implementation of a change. This is not the place to discuss the appropriateness of the change or have overarching design discussions, that should happen on the appropriate [mailing lists].

I have to disagree about the last part - these days because the PR comments go to the mailing list and mailing-list-only comments (in a PR thread) go to the PR, it is indeed the case that design discussions and questions of appropriateness appear in the PR. Personally I prefer JBS to capture such things but the PR has become the one-place-shop in many cases and JBS commentary is typically very limited. Also JBS can't be used by a developer who needs a sponsor and doesn't yet have Author role.

src/guide/reviewing-and-sponsoring-a-change.md line 33:

> 31: You can either reply to existing comments by using the text field beneath each comment, or add a new comment by scrolling to the bottom of the page and use the larger text field where it says "Leave a comment".
> 32: 
> 33: Reviews take time for everyone involved. Your opinions are much appreciated as long as they are on-topic and of a constructive nature. A comment of the form "I don't like this" won't help anyone to improve the change. Instead express what it is that you don't like and give exmaples of how to do it differently. Always be polite and make sure you read through all previous comments before adding your own, the answer you are looking for may already have been given.

Typo: exmaples

I would add a note to check for hidden comments and resolved issues, as they may contain information relevant to your own comment

src/guide/reviewing-and-sponsoring-a-change.md line 35:

> 33: Reviews take time for everyone involved. Your opinions are much appreciated as long as they are on-topic and of a constructive nature. A comment of the form "I don't like this" won't help anyone to improve the change. Instead express what it is that you don't like and give exmaples of how to do it differently. Always be polite and make sure you read through all previous comments before adding your own, the answer you are looking for may already have been given.
> 34: 
> 35: Never ask the author of a change to fix unrelated issues in the code that you find during your review. Such issues should be handled through a separate JBS issue and PR.

Yes and no. :) Unrelated functional fixes are generally not encouraged. Sometimes people will request style or similar fixups "while you are there". On the other hand, many reviewers will object to large scale code cleanups that are not essential to the actual fix and which obscure it. So it is a fine line here.

src/guide/reviewing-and-sponsoring-a-change.md line 43:

> 41: The diff view is pretty straight forward, red stuff has been removed, green stuff has been added. You can configure (among other things) if you want a unified diff or a split view, if you want to hide whitespace changes, what types of files you want to show, and if you want to show the entire change or just what's happened since your last review.
> 42: 
> 43: When you hover the text in the change a blue square with a plus sign pops up to the left of the line. Click it to add a comment about the change on the corresponding line. When you create your first comment you have the option to simply add a single comment or to start a review. In general you want to start a review so that all your comments are gathered in the same context when emails are sent out etc. You can also attach a comment to a file rather than to a specific line in the file by using the speach bubble icon at the top right.

Typo: speach

src/guide/reviewing-and-sponsoring-a-change.md line 67:

> 65: As a sponsor, you aren't technically required to review the change, other Reviewers may already have looked at it or signed up to review. But it will be your name on the commit so you should do whatever you feel is needed to be comfortable with that. You may need to work with the Contributor to make any necessary changes, until you and the Contributor are satisfied with the result, and you are satisfied that the proposed contribution will pass any relevant review and testing. That may take more than one iteration in the worst case. You may also need to help out with any process or practical questions around the OCA, GitHub PRs, and specific requirements in the area or project.
> 66: 
> 67: To integrate the change the Contributor should use the command `/integrate` on the PR as prompted by the bots. Once that is done, you as a sponsor enters the command `/sponsor` in a comment on the PR.

s/enters/enter/

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

PR Review: https://git.openjdk.org/guide/pull/97#pullrequestreview-1572906239
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290815094
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290815476
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290816682
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290819245
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290821132
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290821905
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290822891
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290823173
PR Review Comment: https://git.openjdk.org/guide/pull/97#discussion_r1290824050