github contributor guidelines

2018-02-27 Thread Johan Vos
We now have a Github mirror, and good github projects require clear
guidelines.
Eugene created the following proposal for guidelines, based on standard
guidelines from other projects, but including the link to JBS:

https://github.com/johanvos/openjfxorg/blob/master/CONTRIBUTING.md

Comments welcome of course.

- Johan


Re: Repositories, AdoptOpenJDK and github

2018-02-27 Thread Johan Vos
That is the difficult point indeed.
But why would a PR to OpenJFX be rejected after it was approved in the
github mirror? I would assume the main reason for this is because the PR
did not what it was supposed to do. In that case, it makes sense to remove
the commits from the github mirror as well.

I think the main thing here is that we need to be very serious about
reviewing and accepting PR's in the github mirror. Accepting a PR in github
does not require the *formal* process of creating webrevs etc, but it
requires discussion about the issue with reviewers of OpenJFX.
We have to minimize the number of times an edge case occurs, in which the
discussion was pro PR first, but after it got merged into "development" new
arguments are brought up against the PR.
I think it would be good to have sort of a post-mortem analysis in case
this happens, in order to prevent it from happening again. But as I said,
if it does happen, it probably has good reasons and in that case we have to
remove it from the development branch as well.

I think the more common case would be that an issue is fixed on the github
mirror, but not yet accepted (nor rejected) in OpenJFX, so there will be
some time lag between the PR acceptance and the integration in OpenJFX. But
this should not be a problem, as long as the following scenario is the main
flow:

The github master branch is always synced with OpenJFX, and never gets
modified by other commits.
The github "development" branch is where we accept PR's, that can then be
send upstream. Changes from "master" are regularly merged into
"development". The moment an accepted PR makes it into OpenJFX, it will be
synced into "master" and merged into "development" where the merge happens
silently as there are no conflicts (since development already has this
code).

Does that make sense?

- Johan

On Wed, Feb 28, 2018 at 12:51 AM Kevin Rushforth 
wrote:

>
>
> Nir Lisker wrote:
>
> Johan's thinking was to allow Committers to approve the PR on GitHub --
>> meaning they could be merged on GitHub before an actual Review has
>> happened. Are you proposing to change that?
>
>
> What if the PR is rejected at review? We'll end up with conflicts between
> the repos. And supposed someone works on a different fix and uses the
> rejected PR code, how will that be committed?
>
>
> Good questions; maybe Johan has some thoughts as to how to mitigate this?
>
>
> -- Kevin
>
>
>
> On Wed, Feb 28, 2018 at 12:25 AM, Kevin Rushforth <
> kevin.rushfo...@oracle.com> wrote:
>
>> This seems a good start in formalizing the process. It will need a little
>> tweaking in a couple of areas.
>>
>> Regarding JBS access, even though I want to relax the requirement to
>> become an Author (get a JBS account), it will likely end up somewhere
>> between "an intention to contribute" and "two sponsored contributions,
>> already reviewed and committed". Even without this, there will necessarily
>> be a gap in time between "I want to work on a bug" and getting a JBS
>> account. So there is value in encouraging people to clone the GitHub
>> sandbox, "kick the tires", make a PR to get feedback, etc., before they can
>> access JBS directly (or even while waiting for their OCA to be processed,
>> but no PRs in that case). Something to take into account.
>>
>> Regarding review, we will need a bit more discussion on that. I like the
>> idea of the PR being logged in JBS once it is ready to be reviewed. Johan's
>> thinking was to allow Committers to approve the PR on GitHub -- meaning
>> they could be merged on GitHub before an actual Review has happened. Are
>> you proposing to change that? It might have some advantages, but it could
>> also make it harder in other areas. I'd like to hear from Johan on this.
>> This reminds me that we need to continue the discussion on the general
>> "Review" policy, as it is relevant here.
>>
>> As for whether it is merged into GitHub, I don't have a strong opinion on
>> that. As you say it will be pulled into the mirror anyway (along with
>> changes from reviews happening in JBS that don't first go through the
>> sandbox), so maybe it doesn't matter? On the other hand there might be
>> advantages to getting it into the mainline of the sandbox early? Hard to
>> say.
>>
>> -- Kevin
>>
>>
>> Nir Lisker wrote:
>>
>> Iv'e given the pipeline some thought. I'm purposely ignoring current role
>> names (Author, Contributor...). My suggestions:
>>
>> Potential contributor wants to contribute...
>>
>> 1. Formal process
>>   a. If the issue is not in the JBS, they submit it via bugreport.
>>   b. They send an email on the mailing list regarding the issue (a plan,
>> question on how to approach etc.)
>>   c. If the above effort is "deemed worthy" (whatever that means), and
>> they have signed the OCA, and they then they get access to JBS. If they've
>> given a GitHub account, they get access to GitHub PRs.
>>   d. Discussion from the mailing list is copied/linked to the JBS issue.
>> Maybe if it's their issue (step 

Re: CFV: New OpenJFX Committer: Rajath Kamath

2018-02-27 Thread ankit srivastav
Dear Kevin,

I will get back to you on this shortly with substantial claims.


--Ankit

On 28 Feb 2018 2:23 a.m., "Kevin Rushforth" 
wrote:

> Hi Ankit,
>
> In response to your veto, I took the opportunity to look at the the list
> of changes, and believe that my earlier nomination of Rajath to OpenJFX
> Project Committer was justified, if perhaps barely so.
>
> While there is no objective criteria by which one can say a particular
> changeset is worth 0.5 of a fix, we do often look at 2 to 4 trivial fixes
> or test-only fixes to "make up the difference" in case only 6 or 7 are
> deemed "significant". This is why we usually want 10 or 12 fixes before we
> nominate someone for Committer -- to avoid quibbling over whether one or
> two are worthy of being counted.
>
> Rather than respond to each of your comments individually (although I do
> have one point below), I will instead list the fixes I consider significant.
>
> In looking at the list of fixes again, I would consider the following 7
> non-test fixes to be significant, even though several of them were only a
> few lines of product code changed:
>
> http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/3d5c22069d1f
> http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/5a3cc1b5bb22
> http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/674513271a88
> http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/dc2963c3f7d1 (see
> comment below)
> http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/9f43fb83e989
> http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/dedd5afd753e
> http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/cfa038af148b
>
> In all cases there needed to be an analysis, a fix, and testing to ensure
> that the bug was fixed without introducing a regression. As for your
> assertion about his part of the collaborative fix to upgrade WebKit to
> v605.1, JDK-8187483 (changeset dc2963c3f7d1), you make an unsubstantiated
> claim regarding his contribution. As he did contribute to that fix, I don't
> see any reason to question how significant it was.
>
> In addition to the above 7, and excluding JDK-8185314 (the removal of
> unused files, which I would agree does not count at all), the other three
> test fixes are in my opinion enough justify the nomination.
>
> I would finally point out that Rajath contributed three additional test
> fixes during the two week voting period, for a new total of 14 changesets
> (13 excluding the unused file removal).
>
> Please respond to the list as to whether you feel the additional three
> test fixes, along with my additional explanation, is enough to satisfy your
> concerns over this nomination, and if not, why not. I would like to put the
> nomination forward again for a vote once the objections are resolved.
>
> Thank you.
>
> -- Kevin
>
>
> ankit srivastav wrote:
>
> NO,
>
> Please go through the table, all the points accumulated are not even more
> then 7.
> I have given reasons for my points.
>
>
> *age*
>
> *author*
>
> *description*
>
> Points
>
> Reason
>
> 8 days ago
>
> rkamath
>
> 8196802: 3D unit tests listed as pass  although they are actually skipped
> 
>
> 0.5
>
> Test file, not a direct impact-able code change in product.
>
> 10 days ago
>
> rkamath
>
> 8089454: [HTMLEditor] selection removes CENTER alignment
> 
>
> 0.5
>
> A very small change, why I’m saying so, as the file modified gets called
> directly from the APP written. No debugging/a little is required to make
> the change, which actually defies the purpose of getting knowledge of the
> product.
>
> 13 days ago
>
> rkamath
>
> 8196615: Skip 3D unit tests on system without 3D capability
> 
>
> 0.5
>
> Changes in Test file, not a direct impact-able code change in product.
>
> 4 weeks ago
>
> rkamath
>
> 8165459: HTMLEditor: clipboard toolbar buttons are disabled unexpectedly
> 
>
> 0.5
>
> A very small change, why I’m saying so, as the file modified gets called
> directly from the APP written. No debugging/a little is required to make
> the change, which actually defies the purpose of getting knowledge of the
> product.
>
> 7 weeks ago
>
> rkamath
>
> 8088925: Non opaque background cause NumberFormatException
> 
>
> 0.5
>
> A very small change, why I’m saying so, as the file modified gets called
> directly from the APP written. No debugging/a little is required to make
> the change, which actually defies the purpose of getting knowledge of the
> product.
>
> 2 months ago
>
> rkamath
>
> 8090011: 'tab' key makes control loose focus
> 
> jdk-10+36
>
> 0.5
>
> A very small change, why I’m saying so, as the file mod

Re: [11] Review request : JDK-8195806 : Eliminate dependency on sun.font.lookup in javafx.graphics

2018-02-27 Thread Kevin Rushforth
This looks fine to me. Please add this info to JBS when you get a 
chance, and also link the new bug to this bug.


The fix looks fine to me, although I was puzzled by the following:

-return
-System.getProperty("java.home","") + File.separator +
-"lib" + File.separator + "fonts" + File.separator;
+return System.getProperty("java.home","") + File.separator +
+"lib" + File.separator + "fonts";


Your fix preserves existing behavior, given that the return value from 
the now-eliminated call to the JDKFontLookup class did not have the 
trailing File.separator, but I think that might be a bug given how the 
returned string from this method is used elsewhere in this file. Since 
this is preexisting, you might choose to address that as well in the 
follow-up bug (JDK-8198752). If so, please add a comment to that bug.


+1 from me (you need a +1 from Phil as well)

-- Kevin


Ajit Ghaisas wrote:


Hi Phil,

 

   All my webrev does is to replace the way of obtaining name of JDK 
font directory – and as rightly pointed by you below (first two lines 
of your last reply) – it is what is needed to get rid of dependency on 
sun.font.lookup.


   I have tested it by running an OpenJDK build that doesn't contain a 
lib/fonts directory using Ensemble8 sample.


   

I got your point about Lucida Sans fonts being pointless with 
openJDK & the font finding fallback code is being little dubious.


I would like to address that separately. I have filed bug 
JDK-8198752  to 
handle that.


 


Regards,

Ajit

 


*From:* Phil Race
*Sent:* Thursday, February 15, 2018 10:44 PM
*To:* Ajit Ghaisas
*Cc:* Kevin Rushforth; openjfx-dev@openjdk.java.net
*Subject:* Re: [11] Review request : JDK-8195806 : Eliminate 
dependency on sun.font.lookup in javafx.graphics


 


This part is probably as good as you can do

+return System.getProperty("java.home","") + File.separator +
+"lib" + File.separator + "fonts";


but if we want to support running against OpenJDK which does not have 
Lucida Sans

then we need to look at the caller of getJDKFontDir()

So going forward all of this ..

private static final String jreDefaultFont   = "Lucida Sans Regular";
private static final String jreDefaultFontLC = "lucida sans regular";
private static final String jreDefaultFontFile = "LucidaSansRegular.ttf";

.. is probably pointless.

And if we can't find it then the first layer of fall back code is a 
bit dubious


// Normally use the JRE default font as the last fallback.
// If we can't find even that, use any platform font;
for (String font : fontToFileMap.keySet()) {
String file = findFile(font); // gets full path
fontResource = createFontResource(jreDefaultFontLC, file);
if (fontResource != null) {
break;
}
 
It did not really matter in the past .. it was just to prevent NPE in an unlikely scenario ..

But if it is to be the first class way of finding this font it probably should 
be using
"System" instead ? But then you need to make sure we don't have a circularity 
in the case
that we are using this in initialiasing System.
 
-phil.


 


On 02/14/2018 09:39 PM, Ajit Ghaisas wrote:

Hi Phil,

 


   Thanks for having a look at the changes.

   


   I have little difficulty in understanding your question.

   Are you referring to the sun.font.SunFontManager initialization?

   I am asking as it is not evident from the code changes that I have done 
as part of this webrev.

 


   Request your guidance in this regard.

  


Regards,

Ajit   

 

 


-Original Message-

From: Philip Race 


Sent: Wednesday, February 14, 2018 12:52 PM

To: Ajit Ghaisas

Cc: Kevin Rushforth; openjfx-dev@openjdk.java.net 


Subject: Re: [11] Review request : JDK-8195806 : Eliminate dependency on 
sun.font.lookup in javafx.graphics

 


Well that removes the dependency but how are you fixing the problem of how 
else to find the font ?

There needs to be alternate code or you need to go back to see what would 
happen if some code tried to locate that font and how it would fail.

 


-phil.

 


On 2/13/18, 11:11 PM, Ajit Ghaisas wrote:

Hi Kevin, Phil,

 


   Request you to review following fix :

 


   Issue :  https://bugs.openjdk.java.net/browse/JDK-8195806

 

   Fix :  


http://cr.openjdk.java.net/~aghaisas/fx/8195806/webrev.0/ 


 


Regards,

Ajit

 



Re: Repositories, AdoptOpenJDK and github

2018-02-27 Thread Kevin Rushforth



Nir Lisker wrote:


Johan's thinking was to allow Committers to approve the PR on
GitHub -- meaning they could be merged on GitHub before an actual
Review has happened. Are you proposing to change that?


What if the PR is rejected at review? We'll end up with conflicts 
between the repos. And supposed someone works on a different fix and 
uses the rejected PR code, how will that be committed?


Good questions; maybe Johan has some thoughts as to how to mitigate this?

-- Kevin



On Wed, Feb 28, 2018 at 12:25 AM, Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>> wrote:


This seems a good start in formalizing the process. It will need a
little tweaking in a couple of areas.

Regarding JBS access, even though I want to relax the requirement
to become an Author (get a JBS account), it will likely end up
somewhere between "an intention to contribute" and "two sponsored
contributions, already reviewed and committed". Even without this,
there will necessarily be a gap in time between "I want to work on
a bug" and getting a JBS account. So there is value in encouraging
people to clone the GitHub sandbox, "kick the tires", make a PR to
get feedback, etc., before they can access JBS directly (or even
while waiting for their OCA to be processed, but no PRs in that
case). Something to take into account.

Regarding review, we will need a bit more discussion on that. I
like the idea of the PR being logged in JBS once it is ready to be
reviewed. Johan's thinking was to allow Committers to approve the
PR on GitHub -- meaning they could be merged on GitHub before an
actual Review has happened. Are you proposing to change that? It
might have some advantages, but it could also make it harder in
other areas. I'd like to hear from Johan on this. This reminds me
that we need to continue the discussion on the general "Review"
policy, as it is relevant here.

As for whether it is merged into GitHub, I don't have a strong
opinion on that. As you say it will be pulled into the mirror
anyway (along with changes from reviews happening in JBS that
don't first go through the sandbox), so maybe it doesn't matter?
On the other hand there might be advantages to getting it into the
mainline of the sandbox early? Hard to say.

-- Kevin


Nir Lisker wrote:

Iv'e given the pipeline some thought. I'm purposely ignoring
current role names (Author, Contributor...). My suggestions:

Potential contributor wants to contribute...

1. Formal process
  a. If the issue is not in the JBS, they submit it via bugreport.
  b. They send an email on the mailing list regarding the issue
(a plan, question on how to approach etc.)
  c. If the above effort is "deemed worthy" (whatever that
means), and they have signed the OCA, and they then they get
access to JBS. If they've given a GitHub account, they get access
to GitHub PRs.
  d. Discussion from the mailing list is copied/linked to the JBS
issue. Maybe if it's their issue (step a) then the Reporter field
can change to them.

This ensures that:
* There's 1 entry point.
* GitHub and JBS identities are linked (GitHub identity is verified).
* Being able to comment on JBS is easier - instead of requiring 2
commits it requires good intentions(?)
* Not every person on the planet has access to JBS.

2. Work process
  a. They fork the GitHub repo.
  b. They create a PR with a 2-way link to/from JBS (similar
to  current webrevs - JBS links).
  c. Discussion specifically on the patch should happen in the PR
thread. General info on the bug (affected versions etc.) still
happens in JBS.
  d. After the patch had been reviewed, it is committed to the
Oracle repo. Since GitHub mirrors Oracle I don't think it matters
if the patch is merged into GitHub.

This ensures that:
* It's easier to start working because the GiutHub repo is more
convenient than the Oracle repo currently.
* PRs and JBS issues are mutually aware.
* The submit -> review -> commit process is streamlined.

We pay a synchronization price for having 2 repos and 2 bug
trackers. This is what I could come up with.

- Nir

On Fri, Feb 16, 2018 at 1:14 AM, Kevin Rushforth
mailto:kevin.rushfo...@oracle.com>>
wrote:



Johan Vos wrote:



On Thu, Feb 15, 2018 at 4:09 AM Kevin Rushforth
mailto:kevin.rushfo...@oracle.com>> wrote:

A global reference in JBS would indeed be very good to track
back the work in a PR to a real issue. It can also be very
useful as there are many existing issues in JBS that can be
referred to in future work.

The only issue I see is that in order to create an issue in
JBS, you need to have "author" status, so not everyone can
do this? Given the idea that developers wh

Re: Repositories, AdoptOpenJDK and github

2018-02-27 Thread Nir Lisker
>
> Johan's thinking was to allow Committers to approve the PR on GitHub --
> meaning they could be merged on GitHub before an actual Review has
> happened. Are you proposing to change that?


What if the PR is rejected at review? We'll end up with conflicts between
the repos. And supposed someone works on a different fix and uses the
rejected PR code, how will that be committed?

On Wed, Feb 28, 2018 at 12:25 AM, Kevin Rushforth <
kevin.rushfo...@oracle.com> wrote:

> This seems a good start in formalizing the process. It will need a little
> tweaking in a couple of areas.
>
> Regarding JBS access, even though I want to relax the requirement to
> become an Author (get a JBS account), it will likely end up somewhere
> between "an intention to contribute" and "two sponsored contributions,
> already reviewed and committed". Even without this, there will necessarily
> be a gap in time between "I want to work on a bug" and getting a JBS
> account. So there is value in encouraging people to clone the GitHub
> sandbox, "kick the tires", make a PR to get feedback, etc., before they can
> access JBS directly (or even while waiting for their OCA to be processed,
> but no PRs in that case). Something to take into account.
>
> Regarding review, we will need a bit more discussion on that. I like the
> idea of the PR being logged in JBS once it is ready to be reviewed. Johan's
> thinking was to allow Committers to approve the PR on GitHub -- meaning
> they could be merged on GitHub before an actual Review has happened. Are
> you proposing to change that? It might have some advantages, but it could
> also make it harder in other areas. I'd like to hear from Johan on this.
> This reminds me that we need to continue the discussion on the general
> "Review" policy, as it is relevant here.
>
> As for whether it is merged into GitHub, I don't have a strong opinion on
> that. As you say it will be pulled into the mirror anyway (along with
> changes from reviews happening in JBS that don't first go through the
> sandbox), so maybe it doesn't matter? On the other hand there might be
> advantages to getting it into the mainline of the sandbox early? Hard to
> say.
>
> -- Kevin
>
>
> Nir Lisker wrote:
>
> Iv'e given the pipeline some thought. I'm purposely ignoring current role
> names (Author, Contributor...). My suggestions:
>
> Potential contributor wants to contribute...
>
> 1. Formal process
>   a. If the issue is not in the JBS, they submit it via bugreport.
>   b. They send an email on the mailing list regarding the issue (a plan,
> question on how to approach etc.)
>   c. If the above effort is "deemed worthy" (whatever that means), and
> they have signed the OCA, and they then they get access to JBS. If they've
> given a GitHub account, they get access to GitHub PRs.
>   d. Discussion from the mailing list is copied/linked to the JBS issue.
> Maybe if it's their issue (step a) then the Reporter field can change to
> them.
>
> This ensures that:
> * There's 1 entry point.
> * GitHub and JBS identities are linked (GitHub identity is verified).
> * Being able to comment on JBS is easier - instead of requiring 2 commits
> it requires good intentions(?)
> * Not every person on the planet has access to JBS.
>
> 2. Work process
>   a. They fork the GitHub repo.
>   b. They create a PR with a 2-way link to/from JBS (similar to  current
> webrevs - JBS links).
>   c. Discussion specifically on the patch should happen in the PR thread.
> General info on the bug (affected versions etc.) still happens in JBS.
>   d. After the patch had been reviewed, it is committed to the Oracle
> repo. Since GitHub mirrors Oracle I don't think it matters if the patch is
> merged into GitHub.
>
> This ensures that:
> * It's easier to start working because the GiutHub repo is more convenient
> than the Oracle repo currently.
> * PRs and JBS issues are mutually aware.
> * The submit -> review -> commit process is streamlined.
>
> We pay a synchronization price for having 2 repos and 2 bug trackers. This
> is what I could come up with.
>
> - Nir
>
> On Fri, Feb 16, 2018 at 1:14 AM, Kevin Rushforth <
> kevin.rushfo...@oracle.com> wrote:
>
>>
>>
>> Johan Vos wrote:
>>
>>
>>
>> On Thu, Feb 15, 2018 at 4:09 AM Kevin Rushforth <
>> kevin.rushfo...@oracle.com> wrote:
>>
>> A global reference in JBS would indeed be very good to track back the
>> work in a PR to a real issue. It can also be very useful as there are many
>> existing issues in JBS that can be referred to in future work.
>>
>> The only issue I see is that in order to create an issue in JBS, you need
>> to have "author" status, so not everyone can do this? Given the idea that
>> developers who want to create a PR also need to sign an OCA, it might make
>> sense to somehow combine the administration?
>>
>>
>> I don't think we can combine this, but I hope to be able to relax the
>> requirements to become an Author a little. The current guidelines are 2
>> sponsored contributions [1].
>>
>> Pending 

Re: Repositories, AdoptOpenJDK and github

2018-02-27 Thread Kevin Rushforth
This seems a good start in formalizing the process. It will need a 
little tweaking in a couple of areas.


Regarding JBS access, even though I want to relax the requirement to 
become an Author (get a JBS account), it will likely end up somewhere 
between "an intention to contribute" and "two sponsored contributions, 
already reviewed and committed". Even without this, there will 
necessarily be a gap in time between "I want to work on a bug" and 
getting a JBS account. So there is value in encouraging people to clone 
the GitHub sandbox, "kick the tires", make a PR to get feedback, etc., 
before they can access JBS directly (or even while waiting for their OCA 
to be processed, but no PRs in that case). Something to take into account.


Regarding review, we will need a bit more discussion on that. I like the 
idea of the PR being logged in JBS once it is ready to be reviewed. 
Johan's thinking was to allow Committers to approve the PR on GitHub -- 
meaning they could be merged on GitHub before an actual Review has 
happened. Are you proposing to change that? It might have some 
advantages, but it could also make it harder in other areas. I'd like to 
hear from Johan on this. This reminds me that we need to continue the 
discussion on the general "Review" policy, as it is relevant here.


As for whether it is merged into GitHub, I don't have a strong opinion 
on that. As you say it will be pulled into the mirror anyway (along with 
changes from reviews happening in JBS that don't first go through the 
sandbox), so maybe it doesn't matter? On the other hand there might be 
advantages to getting it into the mainline of the sandbox early? Hard to 
say.


-- Kevin


Nir Lisker wrote:
Iv'e given the pipeline some thought. I'm purposely ignoring current 
role names (Author, Contributor...). My suggestions:


Potential contributor wants to contribute...

1. Formal process
  a. If the issue is not in the JBS, they submit it via bugreport.
  b. They send an email on the mailing list regarding the issue (a 
plan, question on how to approach etc.)
  c. If the above effort is "deemed worthy" (whatever that means), and 
they have signed the OCA, and they then they get access to JBS. If 
they've given a GitHub account, they get access to GitHub PRs.
  d. Discussion from the mailing list is copied/linked to the JBS 
issue. Maybe if it's their issue (step a) then the Reporter field can 
change to them.


This ensures that:
* There's 1 entry point.
* GitHub and JBS identities are linked (GitHub identity is verified).
* Being able to comment on JBS is easier - instead of requiring 2 
commits it requires good intentions(?)

* Not every person on the planet has access to JBS.

2. Work process
  a. They fork the GitHub repo.
  b. They create a PR with a 2-way link to/from JBS (similar 
to  current webrevs - JBS links).
  c. Discussion specifically on the patch should happen in the PR 
thread. General info on the bug (affected versions etc.) still happens 
in JBS.
  d. After the patch had been reviewed, it is committed to the Oracle 
repo. Since GitHub mirrors Oracle I don't think it matters if the 
patch is merged into GitHub.


This ensures that:
* It's easier to start working because the GiutHub repo is more 
convenient than the Oracle repo currently.

* PRs and JBS issues are mutually aware.
* The submit -> review -> commit process is streamlined.

We pay a synchronization price for having 2 repos and 2 bug trackers. 
This is what I could come up with.


- Nir

On Fri, Feb 16, 2018 at 1:14 AM, Kevin Rushforth 
mailto:kevin.rushfo...@oracle.com>> wrote:




Johan Vos wrote:



On Thu, Feb 15, 2018 at 4:09 AM Kevin Rushforth
mailto:kevin.rushfo...@oracle.com>>
wrote:

A global reference in JBS would indeed be very good to track back
the work in a PR to a real issue. It can also be very useful as
there are many existing issues in JBS that can be referred to in
future work.

The only issue I see is that in order to create an issue in JBS,
you need to have "author" status, so not everyone can do this?
Given the idea that developers who want to create a PR also need
to sign an OCA, it might make sense to somehow combine the
administration?


I don't think we can combine this, but I hope to be able to relax
the requirements to become an Author a little. The current
guidelines are 2 sponsored contributions [1].

Pending appointment as an Author, it isn't hard to submit a bug
via http://bugreport.java.com/ . If there is a test case, it
usually gets moved to the JDK project within a day or so (and I
can move them sooner, if needed). The bigger bother is that you
can't comment in JBS on a bug you didn't create, but once the bug
is there, you can work on it in GutHub and/or send email to the
list. I'll also add any comments from contributors who are not yet
Authors to any bug report.

-- Kevin

[1] http://openjdk.java.net/projects/#

Re: Update on enabling JavaFX to work with OpenJDK builds

2018-02-27 Thread Mark Raynsford
On 2018-02-27T13:21:56 -0800
Kevin Rushforth  wrote:
>
> In an earlier email with the subject "javafx might not be present" [1], 
> I mentioned our intention to make it easier for OpenJFX to be built, 
> tested, and run with OpenJDK builds that don't already contain javafx.* 
> modules. This will pave the way to allow a set of pre-built javafx 
> modules to be distributed that will run on top of a pre-built OpenJDK.
> 
> By way of update, this work is underway and can be tracked via the 
> following two JBS issues:

This is good to hear, thank you!

I'll keep an eye on this as it unfolds.

-- 
Mark Raynsford | http://www.io7m.com



Update on enabling JavaFX to work with OpenJDK builds

2018-02-27 Thread Kevin Rushforth
One of the big challenges in running JavaFX with OpenJDK builds is that 
developers need to build OpenJDK locally themselves and include the 
JavaFX bits produced by a locally-built OpenJFX build.


In an earlier email with the subject "javafx might not be present" [1], 
I mentioned our intention to make it easier for OpenJFX to be built, 
tested, and run with OpenJDK builds that don't already contain javafx.* 
modules. This will pave the way to allow a set of pre-built javafx 
modules to be distributed that will run on top of a pre-built OpenJDK.


By way of update, this work is underway and can be tracked via the 
following two JBS issues:


1. Removing internal dependencies:

JDK-8195798 [2] : Address dependencies in javafx.* modules on internal 
APIs of core modules


The above is an umbrella task that points to several linked blocking 
bugs. All of these bugs are in progress and a few are already done 
(e.g., removing jdk.internal.Unsafe from Marlin).



2. Enabling the building, testing, and distribution as a set of separate 
modules


JDK-8198329 [3] : Support FX build / test using JDK that doesn't include 
javafx.* modules


This one depends on the first two, but can be started in parallel, so I 
plan to start on it in the next few days.



Let me know if you have any questions on this.

-- Kevin

[1] 
http://mail.openjdk.java.net/pipermail/openjfx-dev/2018-February/021447.html


[2] https://bugs.openjdk.java.net/browse/JDK-8195798

[3] https://bugs.openjdk.java.net/browse/JDK-8198329



Re: CFV: New OpenJFX Committer: Rajath Kamath

2018-02-27 Thread Kevin Rushforth

Hi Ankit,

In response to your veto, I took the opportunity to look at the the list 
of changes, and believe that my earlier nomination of Rajath to OpenJFX 
Project Committer was justified, if perhaps barely so.


While there is no objective criteria by which one can say a particular 
changeset is worth 0.5 of a fix, we do often look at 2 to 4 trivial 
fixes or test-only fixes to "make up the difference" in case only 6 or 7 
are deemed "significant". This is why we usually want 10 or 12 fixes 
before we nominate someone for Committer -- to avoid quibbling over 
whether one or two are worthy of being counted.


Rather than respond to each of your comments individually (although I do 
have one point below), I will instead list the fixes I consider significant.


In looking at the list of fixes again, I would consider the following 7 
non-test fixes to be significant, even though several of them were only 
a few lines of product code changed:


http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/3d5c22069d1f
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/5a3cc1b5bb22
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/674513271a88
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/dc2963c3f7d1 (see 
comment below)

http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/9f43fb83e989
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/dedd5afd753e
http://hg.openjdk.java.net/openjfx/jfx-dev/rt/rev/cfa038af148b

In all cases there needed to be an analysis, a fix, and testing to 
ensure that the bug was fixed without introducing a regression. As for 
your assertion about his part of the collaborative fix to upgrade WebKit 
to v605.1, JDK-8187483 (changeset dc2963c3f7d1), you make an 
unsubstantiated claim regarding his contribution. As he did contribute 
to that fix, I don't see any reason to question how significant it was.


In addition to the above 7, and excluding JDK-8185314 (the removal of 
unused files, which I would agree does not count at all), the other 
three test fixes are in my opinion enough justify the nomination.


I would finally point out that Rajath contributed three additional test 
fixes during the two week voting period, for a new total of 14 
changesets (13 excluding the unused file removal).


Please respond to the list as to whether you feel the additional three 
test fixes, along with my additional explanation, is enough to satisfy 
your concerns over this nomination, and if not, why not. I would like to 
put the nomination forward again for a vote once the objections are 
resolved.


Thank you.

-- Kevin


ankit srivastav wrote:

NO,

Please go through the table, all the points accumulated are not even 
more then 7.

I have given reasons for my points.


*age*



*author*



*description*



Points



Reason

8 days ago



rkamath



8196802: 3D unit tests listed as pass  although they are actually 
skipped 





0.5



Test file, not a direct impact-able code change in product. 


10 days ago



rkamath



8089454: [HTMLEditor] selection removes CENTER alignment 





0.5



A very small change, why I’m saying so, as the file modified gets 
called directly from the APP written. No debugging/a little is 
required to make the change, which actually defies the purpose of 
getting knowledge of the product.


13 days ago



rkamath



8196615: Skip 3D unit tests on system without 3D capability 





0.5



Changes in Test file, not a direct impact-able code change in product. 


4 weeks ago



rkamath



8165459: HTMLEditor: clipboard toolbar buttons are disabled 
unexpectedly 





0.5



A very small change, why I’m saying so, as the file modified gets 
called directly from the APP written. No debugging/a little is 
required to make the change, which actually defies the purpose of 
getting knowledge of the product.


7 weeks ago



rkamath



8088925: Non opaque background cause NumberFormatException 





0.5



A very small change, why I’m saying so, as the file modified gets 
called directly from the APP written. No debugging/a little is 
required to make the change, which actually defies the purpose of 
getting knowledge of the product.


2 months ago



rkamath



8090011: 'tab' key makes control loose focus 
jdk-10+36




0.5



A very small change, why I’m saying so, as the file modified gets 
called directly from the APP 

Result: New OpenJFX Committer: Rajath Kamath

2018-02-27 Thread Kevin Rushforth

Voting for Rajath Kamath [1] to OpenJFX Committer [2] is now closed.

Yes: 6
Veto: 1
Abstain: 0

According to the Bylaws definition of Lazy Consensus, this nomination 
has failed.


I didn't have a chance to respond to the Veto before the voting period 
ended, but will reply shortly.


-- Kevin

[1] http://openjdk.java.net/census#rkamath
[2] mail.openjdk.java.net/pipermail/openjfx-dev/2018-February/021432.html



Re: Repositories, AdoptOpenJDK and github

2018-02-27 Thread Nir Lisker
Iv'e given the pipeline some thought. I'm purposely ignoring current role
names (Author, Contributor...). My suggestions:

Potential contributor wants to contribute...

1. Formal process
  a. If the issue is not in the JBS, they submit it via bugreport.
  b. They send an email on the mailing list regarding the issue (a plan,
question on how to approach etc.)
  c. If the above effort is "deemed worthy" (whatever that means), and they
have signed the OCA, and they then they get access to JBS. If they've given
a GitHub account, they get access to GitHub PRs.
  d. Discussion from the mailing list is copied/linked to the JBS issue.
Maybe if it's their issue (step a) then the Reporter field can change to
them.

This ensures that:
* There's 1 entry point.
* GitHub and JBS identities are linked (GitHub identity is verified).
* Being able to comment on JBS is easier - instead of requiring 2 commits
it requires good intentions(?)
* Not every person on the planet has access to JBS.

2. Work process
  a. They fork the GitHub repo.
  b. They create a PR with a 2-way link to/from JBS (similar to  current
webrevs - JBS links).
  c. Discussion specifically on the patch should happen in the PR thread.
General info on the bug (affected versions etc.) still happens in JBS.
  d. After the patch had been reviewed, it is committed to the Oracle repo.
Since GitHub mirrors Oracle I don't think it matters if the patch is merged
into GitHub.

This ensures that:
* It's easier to start working because the GiutHub repo is more convenient
than the Oracle repo currently.
* PRs and JBS issues are mutually aware.
* The submit -> review -> commit process is streamlined.

We pay a synchronization price for having 2 repos and 2 bug trackers. This
is what I could come up with.

- Nir

On Fri, Feb 16, 2018 at 1:14 AM, Kevin Rushforth  wrote:

>
>
> Johan Vos wrote:
>
>
>
> On Thu, Feb 15, 2018 at 4:09 AM Kevin Rushforth <
> kevin.rushfo...@oracle.com> wrote:
>
> A global reference in JBS would indeed be very good to track back the work
> in a PR to a real issue. It can also be very useful as there are many
> existing issues in JBS that can be referred to in future work.
>
> The only issue I see is that in order to create an issue in JBS, you need
> to have "author" status, so not everyone can do this? Given the idea that
> developers who want to create a PR also need to sign an OCA, it might make
> sense to somehow combine the administration?
>
>
> I don't think we can combine this, but I hope to be able to relax the
> requirements to become an Author a little. The current guidelines are 2
> sponsored contributions [1].
>
> Pending appointment as an Author, it isn't hard to submit a bug via
> http://bugreport.java.com/ . If there is a test case, it usually gets
> moved to the JDK project within a day or so (and I can move them sooner, if
> needed). The bigger bother is that you can't comment in JBS on a bug you
> didn't create, but once the bug is there, you can work on it in GutHub
> and/or send email to the list. I'll also add any comments from contributors
> who are not yet Authors to any bug report.
>
> -- Kevin
>
> [1] http://openjdk.java.net/projects/#project-author
>
>
> - Johan
>
>


RE: [11] Review request : JDK-8195806 : Eliminate dependency on sun.font.lookup in javafx.graphics

2018-02-27 Thread Ajit Ghaisas
Hi Phil,

 

   All my webrev does is to replace the way of obtaining name of JDK font 
directory – and as rightly pointed by you below (first two lines of your last 
reply) – it is what is needed to get rid of dependency on sun.font.lookup.

   I have tested it by running an OpenJDK build that doesn't contain a 
lib/fonts directory using Ensemble8 sample.

    

I got your point about Lucida Sans fonts being pointless with openJDK & the 
font finding fallback code is being little dubious.

    I would like to address that separately. I have filed bug HYPERLINK 
"https://bugs.openjdk.java.net/browse/JDK-8198752"JDK-8198752 to handle that.

 

Regards,

Ajit

 

From: Phil Race 
Sent: Thursday, February 15, 2018 10:44 PM
To: Ajit Ghaisas
Cc: Kevin Rushforth; openjfx-dev@openjdk.java.net
Subject: Re: [11] Review request : JDK-8195806 : Eliminate dependency on 
sun.font.lookup in javafx.graphics

 

This part is probably as good as you can do

+    return System.getProperty("java.home","") + File.separator +
+    "lib" + File.separator + "fonts";


but if we want to support running against OpenJDK which does not have Lucida 
Sans
then we need to look at the caller of getJDKFontDir() 

So going forward all of this ..

    private static final String jreDefaultFont   = "Lucida Sans Regular";
    private static final String jreDefaultFontLC = "lucida sans regular";
    private static final String jreDefaultFontFile = "LucidaSansRegular.ttf";

.. is probably pointless.

And if we can't find it then the first layer of fall back code is a bit dubious

    // Normally use the JRE default font as the last fallback.
    // If we can't find even that, use any platform font;
    for (String font : fontToFileMap.keySet()) {
    String file = findFile(font); // gets full path
    fontResource = createFontResource(jreDefaultFontLC, file);
    if (fontResource != null) {
    break;
    }
 
It did not really matter in the past .. it was just to prevent NPE in an 
unlikely scenario ..
But if it is to be the first class way of finding this font it probably should 
be using
"System" instead ? But then you need to make sure we don't have a circularity 
in the case
that we are using this in initialiasing System.
 
-phil.

 

On 02/14/2018 09:39 PM, Ajit Ghaisas wrote:

Hi Phil,
 
   Thanks for having a look at the changes.
   
   I have little difficulty in understanding your question.
   Are you referring to the sun.font.SunFontManager initialization?
   I am asking as it is not evident from the code changes that I have done as 
part of this webrev.
 
   Request your guidance in this regard.
  
Regards,
Ajit   
 
 
-Original Message-
From: Philip Race 
Sent: Wednesday, February 14, 2018 12:52 PM
To: Ajit Ghaisas
Cc: Kevin Rushforth; HYPERLINK 
"mailto:openjfx-dev@openjdk.java.net"openjfx-dev@openjdk.java.net
Subject: Re: [11] Review request : JDK-8195806 : Eliminate dependency on 
sun.font.lookup in javafx.graphics
 
Well that removes the dependency but how are you fixing the problem of how else 
to find the font ?
There needs to be alternate code or you need to go back to see what would 
happen if some code tried to locate that font and how it would fail.
 
-phil.
 
On 2/13/18, 11:11 PM, Ajit Ghaisas wrote:

Hi Kevin, Phil,
 
   Request you to review following fix :
 
   Issue :  https://bugs.openjdk.java.net/browse/JDK-8195806
 
   Fix :  
http://cr.openjdk.java.net/~aghaisas/fx/8195806/webrev.0/
 
Regards,
Ajit