Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-18 Thread Kevin Rushforth
On Wed, 18 Dec 2019 18:29:59 GMT, Hadzic Samir  wrote:

>> @Maxoudela the CSR has been approved. Go ahead and `/integrate` this and I 
>> will sponsor it.
> 
> `/integrate`

I think you need to enter the integrate command "as-is" without the 
back-quotes. the `/` needs to be the first character.

-

PR: https://git.openjdk.java.net/jfx/pull/6


Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-18 Thread Hadzic Samir
On Wed, 18 Dec 2019 17:32:05 GMT, Kevin Rushforth  wrote:

>> Looks good.
>> 
>> Once the CSR is approved, this can be integrated.
> 
> @Maxoudela the CSR has been approved. Go ahead and `/integrate` this and I 
> will sponsor it.

`/integrate`

-

PR: https://git.openjdk.java.net/jfx/pull/6


Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-18 Thread Kevin Rushforth
On Mon, 16 Dec 2019 21:32:58 GMT, Kevin Rushforth  wrote:

>> The pull request has been updated with 1 additional commit.
> 
> Looks good.
> 
> Once the CSR is approved, this can be integrated.

@Maxoudela the CSR has been approved. Go ahead and `/integrate` this and I will 
sponsor it.

-

PR: https://git.openjdk.java.net/jfx/pull/6


Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-16 Thread Kevin Rushforth
On Mon, 16 Dec 2019 21:33:11 GMT, Hadzic Samir  wrote:

>> Allright, this is a fix for JDK-8207957
> 
> The pull request has been updated with 1 additional commit.

Looks good.

Once the CSR is approved, this can be integrated.

-

Marked as reviewed by kcr (Lead).

PR: https://git.openjdk.java.net/jfx/pull/6


Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-16 Thread Kevin Rushforth
On Mon, 16 Dec 2019 12:12:31 GMT, Jeanette Winzenburg  
wrote:

>>
> 
> hmm ... think that the bot isn't yet clever enough: this pull request needs 
> approval of two reviewer with _review_ role (mine is only informal)

This one still needs an approved CSR (the CSR is Finalized, but needs to be 
marked as Approved), and I still need to finish my review.

The Skara bot can't know whether all criteria have been met. It can't, for 
example, know whether there are outstanding comments from some reviewers that 
need to be addressed. Nor does it know which PRs need two reviewers (or 
sometimes a third if there is a specific person we would like to review it), 
which ones need a CSR, etc.

So having it state authoritatively that the PR is ready to integrate is a bit 
misleading in this case. This is documented in the Code Review section of the 
[CONTRIBUTING](https://github.com/openjdk/jfx/blob/master/CONTRIBUTING.md) doc:

>NOTE: while the Skara tooling will indicate that the PR is ready to integrate 
>once the first reviewer with a "Reviewer" role in the project has approved it, 
>this may or may not be sufficient depending on the type of fix. For example, 
>you must wait for a second approval for enhancements or high-impact bug fixes.

I wonder if there is a way to improve this?

-

PR: https://git.openjdk.java.net/jfx/pull/6


Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-16 Thread Jeanette Winzenburg
On Mon, 16 Dec 2019 11:46:57 GMT, Ajit Ghaisas  wrote:

>> The pull request has been updated with 1 additional commit.
> 
> 

hmm ... think that the bot isn't yet clever enough: this pull request needs 
approval of two reviewer with _review_ role (mine is only informal)

-

PR: https://git.openjdk.java.net/jfx/pull/6


Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-16 Thread Ajit Ghaisas
On Mon, 16 Dec 2019 11:47:15 GMT, Hadzic Samir  wrote:

>> Allright, this is a fix for JDK-8207957
> 
> The pull request has been updated with 1 additional commit.



-

Marked as reviewed by aghaisas (Reviewer).

PR: https://git.openjdk.java.net/jfx/pull/6


Re: [Rev 07] RFR: 8207957: TableSkinUtils should not contain actual code implementation

2019-12-16 Thread Hadzic Samir
> Allright, this is a fix for JDK-8207957

The pull request has been updated with 1 additional commit.

-

Added commits:
 - 79817025: Remove trailing spaces

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/6/files
  - new: https://git.openjdk.java.net/jfx/pull/6/files/9991ec48..79817025

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/6/webrev.07
 - incr: https://webrevs.openjdk.java.net/jfx/6/webrev.06-07

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jfx/pull/6.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6

PR: https://git.openjdk.java.net/jfx/pull/6