[Kimchi-devel] Why not to use github pull requests?

Daniel Henrique Barboza danielhb at linux.vnet.ibm.com
Thu Aug 6 20:13:38 UTC 2015



On 08/06/2015 04:40 PM, Harshal Patil wrote:
> Any reason for not using pull requests? for me it gives a better 
> interface for the discussion. Better (but very basic) code review tool 
> too.

I do not have anything against pull requests. Feel free to send the pull 
request link for the changes
you want review in the cover page of the plain-text patches.

> Can anyone explain to me what projects like Docker are doing wrong by 
> using pull requests?
>
They're doing nothing wrong. I was a maintainer of a Eclipse Linuxtools 
plug-in and they use pull
requests too. It works.

The reason why Kimchi (and Ginger, for that matter) uses plain-text 
patches in the mailing list
is simplicity. You can easily quote and comment specific parts of the 
patch using standard
email tools. I have kimchi and ginger source code in Power environments 
for testing and more
than once I've sent patches directly from a Power host and reviewed them 
in pine.

Using pull requests for code review implies that I need to open github, 
review the patch
there, comment the patch there and so on. It adds one step and drags 
away the discussing
from the ML. And, unless you want to keep refreshing the pull request 
URL in the browser,
you'll want an email notifying that your changes went upstream. Even if 
github notifies that
automatically to you, other people might be interested in the status of 
those changes too.

TL;DR it is a matter of culture and Kimchi/Ginger was developed under 
the plain-text
git-sendmail paradigm.




>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20150806/4806d04c/attachment.html>


More information about the Kimchi-devel mailing list