Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [glassfish-dev] PR Approval in the GlassFish project
Hi,

On Fri, Feb 28, 2020 at 9:45 PM Bill Shannon <bill.shannon@xxxxxxxxxx> wrote:
I agree with applying only to master.

Many of the changes I see amount to "cleanup".  Review is nice, but for many of these I would be happy for the committer to merge them without review.  Presumably the committer is doing due diligence and making sure that everything builds and runs and at least passes quicklook tests with the proposed change.  Given that, another reviewer doesn't add much for many of these simple changes.

(Unfortunately, I think the merge restrictions are in place and a reviewer is required before the merge is allowed.  Sigh.)

Yes, that is in place, though it can be removed by any GitHub admin.

Jenkins runs a wide selection of tests (quicklook and many from appserv-tests) and you should normally not merge if these don't pass. These same tests can also be run locally. Using the scripts that Romain added a while ago this is quite doable.

Kind regards,
Arjan









 


Steve Millidge (Payara) wrote on 2/25/20 7:57 AM:

Good point. I would assume independent committers are from a separate organization.

 

I don’t want to hinder speed so maybe 1 working day is enough for a single organisation to merge. PRs can always be reverted if people object.

 

Maybe also this rule should also only apply to master rather than feature branches so that feature development can also proceed in the core GitHub repo rather than on forks.

 

Steve

 

From: glassfish-dev-bounces@xxxxxxxxxxx <glassfish-dev-bounces@xxxxxxxxxxx> On Behalf Of Ed Bratt
Sent: 25 February 2020 15:51
To: glassfish developer discussions <glassfish-dev@xxxxxxxxxxx>
Subject: Re: [glassfish-dev] PR Approval in the GlassFish project

 

How about a time allowance if you don't get a review from someone in a separate organization? 48 hours? I presume all committers without a named employer are considered separate organizations. Correct?

 

This will apply to all repositories in Eclipse Glassfish project?

--

Ed Bratt

 

 

-------- Original message --------

From: "Steve Millidge (Payara)" <steve.millidge@xxxxxxxxxxx>

Date: 2/25/20 2:42 AM (GMT-08:00)

To: glassfish developer discussions <glassfish-dev@xxxxxxxxxxx>

Subject: [glassfish-dev] PR Approval in the GlassFish project

 

Hi,

 

As we are starting the development on GlassFish 6.0 I would like to agree how we gain approval of PRs.

 

As a balance between speed and oversight I suggest this rule.

 

“A PR can be merged by any committer (including the creator) after 1 approved review by another committer from a different organisation.”

 

Thoughts?

 

If everybody is OK I will add to the Contributing doc. If there are other suggestions please reply with an alternate suggestion and we can put to the vote.

 

Thanks

 

Steve


_______________________________________________
glassfish-dev mailing list
glassfish-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://urldefense.com/v3/__https://www.eclipse.org/mailman/listinfo/glassfish-dev__;!!GqivPVa7Brio!KwRy8rkpaX3iU77FnvM1ikQvAC-gpcDExo0_pTFFOFp8_YsItk0Hdd8hi-_5nTcb-A$

_______________________________________________
glassfish-dev mailing list
glassfish-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://www.eclipse.org/mailman/listinfo/glassfish-dev

Back to the top