On Tue, 21 May 2024 22:32:21 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 integrating
On Tue, 21 May 2024 22:32:21 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 integrating
On Tue, 21 May 2024 22:32:21 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 integrating
On Tue, 21 May 2024 22:32:21 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 integrating
On Tue, 21 May 2024 22:32:21 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 integrating
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 it
On Tue, 21 May 2024 22:32:21 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 integrating
> 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 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 integrating
On Fri, 17 May 2024 14:55:18 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 integrating
> 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 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 integrating
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 t
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 giv
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
>> comm
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 c
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? :-)
>>
>> ![image](https://github.com/openjdk/jfx/assets/37422899/8daab7cf-f050-4964-b8a6-731666422293)
>>
>> 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 r
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
>> `joha
> 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
>> h
On Thu, 16 May 2024 07:30:31 GMT, Johan Vos wrote:
>> Yeah, that was a typo (which I didn't notice when copying the block from the
>> other doc). I'll fix it. And I agree with your concern, so I'll remove the
>> last sentence.
>
> I agree with the concern, but I still think it's much better to
On Wed, 15 May 2024 21:57:23 GMT, Kevin Rushforth wrote:
>> CONTRIBUTING.md line 233:
>>
>>> 231: * Don't worry too much about import order. Try not to change it but
>>> don't worry about fighting your IDE to stop it from doing so.
>>> 232:
>>> 233: New code should be formatted consistently in
On Wed, 15 May 2024 19:39:10 GMT, Kevin Rushforth wrote:
>> README-code-reviews.md line 66:
>>
>>> 64: * Focus first on substantive comments rather than stylistic comments
>>> 65: * Consider the risk of regression
>>> 66: * Consider any compatibility concerns
>>
>> regression and compatibility
On Wed, 15 May 2024 20:18:05 GMT, Phil Race wrote:
>> This is out of scope for this PR, but it might be something to consider
>> addressing in a follow-on enhancement.
>
> It should be made clear (somewhere, at some point) that it would be rare for
> a CSR to be completed and approved before st
On Wed, 15 May 2024 20:08:05 GMT, John Hendrikx 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 integrating
On Wed, 15 May 2024 19:50:32 GMT, Andy Goryachev 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 re
On Wed, 15 May 2024 20:23:37 GMT, Nir Lisker wrote:
>> Isn't this automatic? Seems weird you could integrate without these passing.
>
>> or is it? :-)
>
> ![image](https://github.com/openjdk/jfx/assets/37422899/8daab7cf-f050-4964-b8a6-731666422293)
>
> Looks to me like it is...
A passing GHA t
On Wed, 15 May 2024 20:12:09 GMT, Phil Race wrote:
>> README-code-reviews.md line 48:
>>
>>> 46: All code reviews must be done via a pull request submitted against this
>>> GitHub repo, [openjdk/jfx](https://github.com/openjdk/jfx). A JBS bug ID
>>> must exist before the pull request will be r
On Wed, 15 May 2024 19:04:19 GMT, Nir Lisker 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 integrating
>>
On Wed, 15 May 2024 20:33:37 GMT, Andy Goryachev wrote:
>> He means people reading reviews are usually on github and your github ID is
>> kevinrusforth,
>> so they might be confused by "kcr" which is your openjdk id.
>>
>> Since the context is identifying OpenJDK Project leads, I think the Open
On Wed, 15 May 2024 20:07:06 GMT, Phil Race 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 integrating
>> c
On Wed, 15 May 2024 17:45:56 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 integrating
> c
On Wed, 15 May 2024 20:03:01 GMT, Phil Race wrote:
>> Not sure what you mean, but it doesn't seem related to this PR.
>
> He means people reading reviews are usually on github and your github ID is
> kevinrusforth,
> so they might be confused by "kcr" which is your openjdk id.
>
> Since the con
On Wed, 15 May 2024 20:08:10 GMT, John Hendrikx wrote:
>> or is it? :-)
>
> Isn't this automatic? Seems weird you could integrate without these passing.
> or is it? :-)
![image](https://github.com/openjdk/jfx/assets/37422899/8daab7cf-f050-4964-b8a6-731666422293)
Looks to me like it is...
---
On Wed, 15 May 2024 19:00:57 GMT, Nir Lisker 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 integrating
>>
On Wed, 15 May 2024 19:32:04 GMT, Kevin Rushforth wrote:
>> README-code-reviews.md line 103:
>>
>>> 101: To ensure that new features are consistent with the rest of the API
>>> and the desired direction of the Project, a CSR is required for a new
>>> Feature, API addition, or behavioral change
On Wed, 15 May 2024 17:45:56 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 integrating
> c
On Wed, 15 May 2024 17:45:56 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 integrating
> c
On Wed, 15 May 2024 19:53:43 GMT, Andy Goryachev wrote:
>> README-code-reviews.md line 68:
>>
>>> 66: * Consider any compatibility concerns
>>> 67: * Check whether there is an automated test; if not, ask for one, if it
>>> is feasible
>>> 68: * Make sure that the PR has executed the GHA tests a
On Wed, 15 May 2024 18:56:29 GMT, Kevin Rushforth wrote:
>> README-code-reviews.md line 10:
>>
>>> 8:
>>> 9: __Project Co-Lead__: Kevin Rushforth (kcr)
>>> 10: __Project Co-Lead__: Johan Vos (jvos)
>>
>> There are two sets of ids - one for OpenJFX/JBS and one for Github. This
>> might be co
On Wed, 15 May 2024 19:23:25 GMT, Nir Lisker 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 integrating
>>
On Wed, 15 May 2024 19:16:31 GMT, Nir Lisker 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 integrating
>>
On Wed, 15 May 2024 17:45:56 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 integrating
> c
On Wed, 15 May 2024 18:20:22 GMT, Andy Goryachev 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 integrating
On Wed, 15 May 2024 17:45:56 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 integrating
> c
On Wed, 15 May 2024 18:05:10 GMT, Andy Goryachev 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 integrating
On Wed, 15 May 2024 17:45:56 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 integrating
> c
On Wed, 15 May 2024 17:45:56 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 integrating
> c
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
Policies](htt
56 matches
Mail list logo