Set review required for Pull Requests

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Set review required for Pull Requests

Ronny Trommer-3
Hi Guys,

I would like vote for setting up Review required [1] for protected branches like foundation / foundation-2016 / master / develop?

Any thoughts about this?


Greetings Ronny

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel
Reply | Threaded
Open this post in threaded view
|

Re: Set review required for Pull Requests

Jesse White-3

This sounds like good practice to me.

I don't see any problems with addressing multiple issues in a single pull request as long as the commits are related and properly identified.

-Jesse

On 01/30/2017 08:53 AM, Ronny Trommer wrote:
Hi Guys,

I would like vote for setting up Review required [1] for protected branches like foundation / foundation-2016 / master / develop?

Any thoughts about this?


Greetings Ronny


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot


_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel
Reply | Threaded
Open this post in threaded view
|

Re: Set review required for Pull Requests

Jesse White-3
Oups, this was meant for the other thread...

On 01/31/2017 09:54 AM, Jesse White wrote:

> This sounds like good practice to me.
>
> I don't see any problems with addressing multiple issues in a single pull request as long as the commits are related and
> properly identified.
>
> -Jesse
>
> On 01/30/2017 08:53 AM, Ronny Trommer wrote:
>> Hi Guys,
>>
>> I would like vote for setting up Review required [1] for protected branches like foundation / foundation-2016 / master
>> / develop?
>>
>> Any thoughts about this?
>>
>> [1] https://help.github.com/articles/about-required-reviews-for-pull-requests/
>>
>> Greetings Ronny
>>
>>
>> ------------------------------------------------------------------------------
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>>
>>
>> _______________________________________________
>> Please read the OpenNMS Mailing List FAQ:
>> http://www.opennms.org/index.php/Mailing_List_FAQ
>>
>> opennms-devel mailing list
>>
>> To *unsubscribe* or change your subscription options, see the bottom of this page:
>> https://lists.sourceforge.net/lists/listinfo/opennms-devel
>

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel
Reply | Threaded
Open this post in threaded view
|

Re: Set review required for Pull Requests

Jesse White-3
In reply to this post by Ronny Trommer-3

Don't we already need "write" permissions to be able to merge a PR? If so, what's to gain by this extra step?

-Jesse


On 01/30/2017 08:53 AM, Ronny Trommer wrote:
Hi Guys,

I would like vote for setting up Review required [1] for protected branches like foundation / foundation-2016 / master / develop?

Any thoughts about this?


Greetings Ronny


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot


_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel
Reply | Threaded
Open this post in threaded view
|

Re: Set review required for Pull Requests

Markus von Rüden
In reply to this post by Ronny Trommer-3
Branches are already protected and can only be written to by committers with the according rights.

We already do pull requests and reviews. 
There is no point to add more complexity and remove flexibility, in case we need it, from my point of view.

- Markus


On 30 Jan 2017, at 14:53, Ronny Trommer <[hidden email]> wrote:

Hi Guys,

I would like vote for setting up Review required [1] for protected branches like foundation / foundation-2016 / master / develop?

Any thoughts about this?


Greetings Ronny
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Please read the OpenNMS Mailing List FAQ:
http://www.opennms.org/index.php/Mailing_List_FAQ

opennms-devel mailing list

To *unsubscribe* or change your subscription options, see the bottom of this page:
https://lists.sourceforge.net/lists/listinfo/opennms-devel