On Sat, 18 May 2024 14:28:31 GMT, Nir Lisker wrote:
>> We have by now cleaned up our public API to avoid classes with an implicit
>> no-arg constructor, so the only way this situation could arise in the future
>> is if someone adds a new public class, which needs a CSR anyway.
>>
>> I guess
On Sat, 18 May 2024 14:24:49 GMT, Kevin Rushforth wrote:
> Maybe it's worth changing "adds any new" to "adds, removes, or modifies any"?
This looks like a good idea.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605798488
On Fri, 17 May 2024 15:02:14 GMT, Nir Lisker wrote:
>> Kevin Rushforth has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 20 additional
>> commits
On Fri, 17 May 2024 14:58:41 GMT, Nir Lisker wrote:
>> Kevin Rushforth has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 20 additional
>> commits
On Fri, 17 May 2024 14:10:43 GMT, Kevin Rushforth wrote:
>> Update the code review guidelines for JavaFX.
>>
>> The JavaFX
>> [CONTRIBUTING](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/CONTRIBUTING.md)
>> guidelines includes guidance for creating, reviewing, and
On Fri, 17 May 2024 14:10:43 GMT, Kevin Rushforth wrote:
>> Update the code review guidelines for JavaFX.
>>
>> The JavaFX
>> [CONTRIBUTING](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/CONTRIBUTING.md)
>> guidelines includes guidance for creating, reviewing, and
On Wed, 15 May 2024 22:18:24 GMT, Kevin Rushforth wrote:
>> README-code-reviews.md line 79:
>>
>>> 77: * All Reviewers who have requested the chance to review have done so
>>> (or indicated that they are OK with it going in without their review). In
>>> rare cases a Project Lead may override
On Thu, 16 May 2024 07:27:34 GMT, Johan Vos wrote:
>> That's a good idea.
>
> In the ideal world where we have tons of regression and compatibility tests,
> I would agree. Unfortunately, we are totally not there yet. Compared to other
> projects, the quality of tests in OpenJFX is good, but
On Wed, 15 May 2024 18:30:06 GMT, Andy Goryachev wrote:
>> Kevin Rushforth has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains 20 additional
>>
On Wed, 15 May 2024 19:38:55 GMT, Kevin Rushforth wrote:
>> README-code-reviews.md line 64:
>>
>>> 62:
>>> 63: * Make sure you understand why there was an issue to begin with, and
>>> why/how the proposed PR solves the issue
>>> 64: * Focus first on substantive comments rather than stylistic
On Wed, 15 May 2024 22:00:42 GMT, Kevin Rushforth wrote:
>> I think it worth noting that in skara syntax that isn't two people with the
>> reviewer role.
>> And tell people what to use if that is what they intend - eg if I have it
>> right
>
> It's `/reviewers 2 reviewers`, so I'll add that as
On Wed, 15 May 2024 21:58:46 GMT, Kevin Rushforth wrote:
>> README-code-reviews.md line 14:
>>
>>> 12: ### Reviewers
>>> 13:
>>> 14: The [List of Reviewers](https://openjdk.java.net/census#openjfx) is on
>>> the OpenJDK Census.
>>
>> We use ".org" now, not ".java.net"
>
> Yes, I missed this.
On Wed, 15 May 2024 22:13:34 GMT, Kevin Rushforth wrote:
>>> or is it? :-)
>>
>> 
>>
>> Looks to me like it is...
>
> A passing GHA test run is neither necessary nor sufficient. It is an
> interesting
On Wed, 15 May 2024 22:10:12 GMT, Kevin Rushforth wrote:
> > ...inadvertent introduction of new API (that will have to be deprecated if
> > missed)
>
> I think this is worth mentioning.
Fixed.
-
PR Review Comment: https://git.openjdk.org/jfx/pull/1455#discussion_r1605063298
On Wed, 15 May 2024 22:05:46 GMT, Kevin Rushforth wrote:
>> README-code-reviews.md line 60:
>>
>>> 58: * If you want an area expert to review a PR, indicate this in a comment
>>> of the form: `Reviewers: @PERSON1 @PERSON2`; the requested reviewers can
>>> indicate whether or not they plan to
On Wed, 15 May 2024 21:57:55 GMT, Kevin Rushforth wrote:
>> exactly my point. when the majority of time is spent in the context of
>> github (PR review etc) it might be confusing when one types `@jvos` and it's
>> not found, or when within a PR there are references to both `jvos` and
>>
> Update the code review guidelines for JavaFX.
>
> The JavaFX
> [CONTRIBUTING](https://github.com/kevinrushforth/jfx/blob/8332313-contributing/CONTRIBUTING.md)
> guidelines includes guidance for creating, reviewing, and integrating
> changes to JavaFX, along with a pointer to a [Code Review
On Thu, 16 May 2024 08:48:14 GMT, John Hendrikx wrote:
>> I agree with the concern, but I still think it's much better to encourage
>> developers to do formatting in a separate issue (or not at all) with all the
>> required administration, than to sneak in a formatting change in a PR that
>>
18 matches
Mail list logo
