RFR: Removing a JBS Issue [v5]

[Alexey Ivanov]

On Wed, 16 Aug 2023 08:36:12 GMT, Jesper Wilhelmsson <jwilhelm at openjdk.org> wrote:

>> src/guide/working-with-pull-requests.md line 15:
>> 
>>> 13: ## Think once more
>>> 14: 
>>> 15: All code reviews in OpenJDK are done in public. Once you open your PR for public review the internet can see it and comment on it. Make sure your code is ready for it. Look through your comments, make sure that temporary code is gone, and make sure you have sanitized your method and variable names. Also, make sure you understand your code. Why is it working? What are the potential pitfalls? What are the edge-cases? If you haven't already answered all these questions in the mail conversation that preceded this PR, it's likely that you will need to answer them during the review.
>> 
>> Suggestion:
>> 
>> All code reviews in OpenJDK are done in public. Once you open your PR for public review, the internet can see it and comment on it. Make sure your code is ready for it. Look through your comments, make sure that temporary code is gone, and make sure you have sanitized your method and variable names. Also, make sure you understand your code. Why is it working? What are the potential pitfalls? What are the edge-cases? If you haven't already answered all these questions in the mail conversation that preceded this PR, it's likely that you will need to answer them during the review.
>> 
>> Should we add a comma to separate the sentences?
>
> That doesn't read right to me. Those aren't two different sentences.

These are not two independent sentences but these are two dependent clauses: the first one is a conditional clause that is followed by the main clause. Without the comma between ‘review’ and ‘the internet’, one could interpret that ‘the internet’ starts an adjective clause that describes *the review* further, which is not the case. That's exactly what happened to me when I read the entire paragraph thoroughly. With the comma in place, the confusion goes away, the reader has no chance of misinterpreting the sentence: the conditional clause that starts the sentence is explicitly separated from the start of main clause.

In this case, _‘once’_ is a conjunction, similar to _‘if’_, and you've got a comma after the conditional sentence where it goes before the main clause in the last sentence of this very paragraph:

- Once (if) you…, the internet can see…
- If you…, it is likely…

If a condition or an adverbial goes before the subject, there's usually a comma.

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

PR Review Comment: https://git.openjdk.org/guide/pull/112#discussion_r1295952001