← Back to team overview

openerp-community team mailing list archive

Re: "Merge pull request" considered harmful

 

IMHO, what maintainers did on Launchpad was to just to merge, thus
creating one new commit including all the changes in the feature
branch. In addition to that, we could to small modifications ourselves
before pushing to Launchpad.

On the other hand, git makes it possible to do things that we would
not even dream to do practically with bazaar: an interactive rebase
allows to clean up history doing things like

- rebasing the history, making it look like history is more linear
than it was at the start
- clean up history by squashing old commits together
- get rid of commits that break the automatic tests, allowing historic
operations like bisect and benchmarks on the history
- change past commit messages to make them better

and so on.

Or we can just use the "merge" button on github, that does the same
thing we did manually on Launchpad (clearly without the possibility to
make small changes ourselves).

So IMO it's good to have some guidelines to contributors (i.e. try to
produce a clean history in their branch) but I would wait a bit before
forcing maintainers to have a complex workflow in all cases.

In the meantime we can, for example, try to ask kindly to contributors
to improve their feature branches themselves, and use git-rebase-fu
only when necessary.

On Thu, Jul 3, 2014 at 9:02 AM, Joël Grand-Guillaume
<joel.grandguillaume@xxxxxxxxxxxxxx> wrote:
> I think, after reading carefully, that this is more important than it seems.
> By increasing the number of project, it's important that such rule are
> given, even if not a constraint for now. But at least we should let people
> now about.
>
> So I'm in favor of writing this in the docs and howto's. Committers should
> now that IMO.
>
> Improving the quality is one of our goal, even if not applicable eveytime by
> lack of time, it should be our goal.
>
>
>
>
> On Thu, Jul 3, 2014 at 12:05 AM, Nhomar Hernández <nhomar@xxxxxxxxx> wrote:
>>
>> Pedro.
>>
>> It is a "good to have" but I think we can just pull locally test and then
>> push.
>>
>> If you do this process recommended it ia good too! But IMHO Not the rule
>> for now.
>>
>> Be careful! The pr button is dangerous without a correct CIS configured.
>>
>> Written from my android
>>
>> On Jul 2, 2014 3:16 PM, "Pedro Manuel Baeza Romero"
>> <pedro.baeza@xxxxxxxxx> wrote:
>>>
>>> We have to set new contributing HowTos for working with GitHub, and this
>>> article points out some troubles with Merge PR button, but the way it says
>>> to merge PR can be very laborious for maintainers. Can we assume this amount
>>> of extra work? One of the highlights we talk about when the switch to GitHub
>>> was the Merge PR button.
>>>
>>> Regards.
>>>
>>>
>>> 2014-06-27 14:23 GMT+02:00 Tymoteusz Motylewski
>>> <t.motylewski@xxxxxxxxxxxx>:
>>>>
>>>> +1 for the clean git history. Merge commits are painful.
>>>> With clean commit messages and linear history things like changelog can
>>>> be easily generated automatically. And by changelog I mean sth which really
>>>> gives the reader some knowledge.
>>>> See other OSS project TYPO3 Neos for example of such a history
>>>>
>>>> http://docs.typo3.org/neos/TYPO3NeosDocumentation/Appendixes/ChangeLogs/102.html
>>>>
>>>>
>>>>
>>>>
>>>> 2014-06-27 13:51 GMT+02:00 Lionel Sausin <ls@xxxxxxxxxxxxxxxx>:
>>>>
>>>>> Very interesting reading. I totally support this point of view even if
>>>>> I'm only a contributor - clean history is hours not lost searching for
>>>>> stuff. Bisect, annotate, etc all become easier.
>>>>>
>>>>> Le 27/06/2014 12:10, Alexandre Fayolle a écrit :
>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> I found this blog post both interesting and useful :
>>>>>>
>>>>>> http://blog.spreedly.com/2014/06/24/merge-pull-request-considered-harmful
>>>>>>
>>>>>> Leonardo Pistone had shown me https://github.com/github/hub before and
>>>>>> it's a nice tool. And since odoo and the community addons are moving
>>>>>> to
>>>>>> a new platform, I thought I'd share this piece of insight with all of
>>>>>> you.
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Mailing list: https://launchpad.net/~openerp-community
>>>>> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
>>>>> Unsubscribe : https://launchpad.net/~openerp-community
>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>>
>>>> Pozdrawiam serdecznie / Best regards
>>>> Tymoteusz Motylewski
>>>> Head of development
>>>> +48 502 472 202
>>>>
>>>> t.motylewski@xxxxxxxxxxxx
>>>>
>>>> http://www.macopedia.pl/
>>>> Ul. Roosevelta 5/8
>>>> 60-829 Poznań
>>>> Tel. +48 61 622 96 95
>>>> Fax +48 61 623 24 48/
>>>> Macopedia Spółka z ograniczoną odpowiedzialnością, z siedzibą ul.
>>>> Roosevelta 5/8; 60-829 Poznań, wpisana do Krajowego Rejestru Sądowego,
>>>> prowadzonego przez Sąd Rejonowy, VIII Wydział Gospodarczy, KRS
>>>> 0000416744, o opłaconym kapitale zakładowym w wysokości 5.000,00 zł, o
>>>> numerze NIP: 781-187-78-41, numerze Regon 302076699
>>>>
>>>> _______________________________________________
>>>> Mailing list: https://launchpad.net/~openerp-community
>>>> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~openerp-community
>>>> More help   : https://help.launchpad.net/ListHelp
>>>>
>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~openerp-community
>>> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~openerp-community
>>> More help   : https://help.launchpad.net/ListHelp
>>>
>>
>> _______________________________________________
>> Mailing list: https://launchpad.net/~openerp-community
>> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~openerp-community
>> More help   : https://help.launchpad.net/ListHelp
>>
>
>
>
> --
>
>
> camptocamp
> INNOVATIVE SOLUTIONS
> BY OPEN SOURCE EXPERTS
>
> Joël Grand-Guillaume
> Division Manager
> Business Solutions
>
> +41 21 619 10 28
> www.camptocamp.com
>
>
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-community
> Post to     : openerp-community@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~openerp-community
> More help   : https://help.launchpad.net/ListHelp
>


Follow ups

References