Contributors mailing list archives

contributors@odoo-community.org

Browse archives

Avatar

Re: OCA Code sprint: sprint topics

by
Open for Small Business, Graeme Gellatly
- 19/09/2017 02:36:21
There is a standard github syntax for limiting diff, something like

https://github.com/OCA/server-tools/compare/f00e044...6d08d5d - a real link for demonstration, although a somewhat pointless one, just took first PR with 2 commits.
Or just clicking commits on the PR and selecting the commits to review.

So submitter can simply filter-branch (or whatever method to preserve history) up to the last commit in prior version and note the SHA. I'm sure there are easier ways, like tag or something as well.  But the point is provided when we do port we carry history, large diffs should be a non-issue.

On Wed, Sep 13, 2017 at 3:23 AM, Ronald Portier <ronald@therp.nl> wrote:
I think it will not be to difficult when migrating a module to first
bring over the branch with its history as is. And then have the commits
to adapt the module to the new version.

The review can then concentrate on the later commits, that should not
have to big diffs.

Kind regards, Ronald

Op 12-09-17 om 17:08 schreef Dave Lasley:
> To start - I like a clean branch and commit history.
> 
> That said, I think this will create complication and more overhead from
> a reviewer and migrator perspective. The reasoning behind this is that
> now all of the commits are being reviewed as if this is a new module, vs
> just reviewing the diff to make sure the migration was performed
> correctly. This could mean the difference of PRs that are 20 files and
> PRs that are 3. 
> 
> Further - with the modules being reviewed as new again, there will be a
> lot more changes requested on the module which will add overhead to the
> person submitting the PR.
> 
> Thank you,
> Dave Lasley - LasLabs
> /Founder / CEO/
> /dave@laslabs.com/ <mailto:dave@laslabs.com>
> /+1 (702) 508-8894 /
> /+1 (888) 678-6313;5 /
> 
>  src="cid:B6781F39-FEE7-4B4E-91AD-FDB83E4A18E3@dlasley.net" class="">
> 
> 
>> On Sep 12, 2017, at 7:39 AM, Paul Catinean <paulcatinean@gmail.com
>> <mailto:paulcatinean@gmail.com>> wrote:
>>
>> I also think a clean start is better, much less confusion and overhead +1
>>
>> On Tue, Sep 12, 2017 at 5:23 PM, Ronald Portier <ronald@therp.nl
>> <mailto:ronald@therp.nl>> wrote:
>>
>>     +1 For starting with an empty branch and bringing the commits on migration.
>>
>>     Op 12-09-17 om 16:08 schreef Pedro M. Baeza (Tecnativa):
>>     > Maxime, I think you wanted to say v10 installable modules.
>>     > 
>>     > My vote is for removing all modules and start from scratch, bringing the
>>     > commits when migrating the module. This way, we can create the branches
>>     > in any moment without waiting for all the 10 PRs still waiting to be
>>     > reviewed/merged.
>>     > 
>>     > Regards.
>>     > 
>>     > 2017-09-12 15:53 GMT+02:00 Maxime Chambreuil
>>     > :
>>     > 
>>     >     +1 for v10 modules only with installable = False
>>     > 
>>     >     Ursa Information Systems 	*Maxime Chambreuil
>>     >     */Project Manager / Consultant/
>>     > 
>>     >     Ursa Information Systems
>>     >     1450 W Guadalupe Road, Suite 132
>>     >     
>>     >     Gilbert, Arizona, 85233
>>     > 
>>     >     /Office: 1
>>     -855-URSA ERP x 710
>>     >                     1-855-877 2377 x 710 
>>     >     Mobile:   1-602-427-5632 / 	
>>     > 
>>     > 
>>     >     On Tue, Sep 12, 2017 at 5:23 AM, Pedro M. Baeza (Tecnativa)
>>     >      wrote:
>>     > 
>>     >         No, not installable modules are not "reserved" in Odoo Apps for
>>     >         the first coming. The module name is taken when the daily
>>     >         scanner checks that is installable, so indeed that argument
>>     >         doesn't serve.
>>     > 
>>     >         Regards.
>>     > 
>>     >         2017-09-12 12:08 GMT+02:00 Sylvain GARANCHER (SYLEAM)
>>     >         :
>>     > 
>>     >             I'm in favor of creating an empty branch for each new
>>     >             version, and add the modules one by one in the migration PRs :
>>     >             - We don't lose history, thanks to our migration procedure.
>>     >             - Each branch will contain only available modules : No more
>>     >             issues/questions about not installable modules or modules
>>     >             not working because the user just changed the installable
>>     >             flag to make it appear in Odoo.
>>     >             - Each branch will contain only currently available modules
>>     >             commits : history is easier to read, the repo is smaller for
>>     >             people who use shallow and/or single branch cloning.
>>     >             - When migrating a module that has not been migrated for the
>>     >             previous version, we directly take the last working version
>>     >             : I already saw PRs for 10.0 based on the 9.0 branch, but
>>     >             the module was not migrated to 9.0, and the 8.0 version had
>>     >             bugfixes in the meantime.
>>     >             - Cherry-picking bugfixes will work, even if the module
>>     >             never existed before in this branch, so this is not a problem.
>>     >             - We can review only the diff of the migration (either using
>>     >             GitHub or our local git).
>>     >             - We already have modules in some branches that are not in
>>     >             the latest branch (modules added at version X-1 after the
>>     >             verison X branch was created), so not seeing all modules in
>>     >             the latest branch is not a /new/ issue. And it will be more
>>     >             consistent here : currently, we are between "all modules are
>>     >             in the latest branch" and "only available modules are in the
>>     >             latest branch".
>>     > 
>>     >             I think the reason raised by Graeme is not an issue : "On
>>     >             the more cynical side, to stop unscrupulous 3rd parties
>>     >             pinching module names for app store and/or just porting and
>>     >             claiming as their own."
>>     >             Odoo already helped us in this way (OCA's modules included
>>     >             in proprietary wensite themes, for example). But I don't
>>     >             know for modules that have the same name, but are completely
>>     >             different...
>>     >             Also, how does it work currently ? Does Odoo Apps list
>>     >             applications that are not installable ?
>>     > 
>>     >             Cordialement,
>>     >             --
>>     > 
>>     > 
>>     > 
>>     >             2017-09-12 11:23 GMT+02:00 Andreas Stauder
>>     >             <andreas.stauder@braintec-group.com
>>     <mailto:andreas.stauder@braintec-group.com>
>>     >             <mailto:andreas.stauder@braintec-group.com
>>     <mailto:andreas.stauder@braintec-group.com>>>:
>>     > 
>>     >                 What is about fixing bugs or adding features in an
>>     >                 earlier version (e.g. 8.0)? We do cherry-picking and
>>     >                 create a new PR for 9.0 and 10.0, right? Does this still
>>     >                 work when a addon is removed because it was not ported
>>     >                 in two versions, but will added again for a later
>>     >                 version? On the other side: When a module was dead for
>>     >                 two versions it should stayed buried and replaced by a
>>     >                 complete new one (especially with the API change since
>>     >                 then).
>>     > 
>>     >                 So I vote for removing addons not ported since 2
>>     >                 versions, but keeping the other ones and setting them
>>     >                 uninstallable as it was done for 10.0. 
>>     > 
>>     >                 Regards
>>     >                 Andreas
>>     > 
>>     > 
>>     > 
>>     >                 On Tue, Sep 12, 2017 at 10:38 AM, Eric Caudal
>>     >                 >     >                 > wrote:
>>     > 
>>     >                     +1
>>     > 
>>     >                     --
>>     >                     *Eric Caudal* /[Founder and CEO]/
>>     >                     Skype: elico.corp. Phone: + 86 186 2136 1670  (Cell),
>>     >                     + 86 21 6211 8017/27/37 (Office)
>>     >                     *Elico Shanghai* (Hong Kong/Shenzhen/Singapore)
>>     >                     
>>     >                     /*Odoo Gold Partner // Best Odoo Partner APAC 2014
>>     >                     and 2016*/
>>     >                     On 09/12/2017 03:53 PM, Jairo Llopis wrote:
>>     >                     
>>>     cite="mid:CAP5xMq=oXDT7Dwg_XuOOvZ+81GWsO90TV4gCDcJF8nKU=cPbig@mail.gmail.com
>>>     <mailto:cPbig@mail.gmail.com> > <mailto:cPbig@mail.gmail.com
>>>     <mailto:cPbig@mail.gmail.com>>"> > I'd personally love to see the
>>>     11.0 branch > completely empty of uninstallable addons, and grow
>>>     > as they become migrated. > > The git size problem is not a
>>>     problem for me > anymore, because since long ago I got used to
>>>     always > clone shallow repositories (git clone --depth 100 >
>>>     $repo), so the size does not matter much. > > Our migration
>>>     instructions take care of keeping > history, and you can review
>>>     only the migration > commits in a PR nowadays, so the little diff
>>>     in PRs > is not a need anymore. > > This would remove garbage
>>>     code and the need to > explain many "whys". > > Just my 2 cents
>>>     :) > > -- > Jairo Llopis > >
>>>     _______________________________________________ > Mailing-List: >
>>>     https://odoo-community.org/groups/contributors-15
>>>     > > Post to:
>>>     mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org> >
>>>     <mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org>> > Unsubscribe: >
>>>     https://odoo-community.org/groups?unsubscribe
>>>      > > > >
>>>     _______________________________________________ > Mailing-List: >
>>>     https://odoo-community.org/groups/contributors-15
>>>      > > Post to:
>>>     mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org> >
>>>     <mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org>> > Unsubscribe: >
>>>     https://odoo-community.org/groups?unsubscribe
>>>      > > > > > > -- >
>>>     brain-tec Group > > +41 (0) 44 552 01 20 
>>>     20> www.braintec-group.com  >
>>>     Technoparkstrasse 1 ▪ CH-8005 Zürich
>>>     
>>>     > > Andreas Stauder > Head of Development, Consultant und
>>>     Projektleiter > > Certified Professional Scrum Master > lic.iur.
>>>     > andreas.stauder@braintec-group.com
>>>     <mailto:andreas.stauder@braintec-group.com> >
>>>     <mailto:andreas.stauder@braintec-group.com
>>>     <mailto:andreas.stauder@braintec-group.com>> > > *brain-tec AG* >
>>>     Technoparkstrasse 1
>>>     
>>>     > > CH-8005 Zürich *brain-tec AG* > Saflischstrasse 4 > > CH-3900
>>>     Brig
>>>     
>>>     > > *brain-tec AG* > Rue de la Madeleine 38 > CH-1963 Vétroz
>>>     *brain-tec Germany GmbH* > Otto-Lilienthal-Straße 36
>>>     
>>>     > > D-71034 Böblingen *brain-tec Spain S.L.* > Calle Velazquez
>>>     138
>>>     
>>>     > > ES-28006 Madrid > > Best Odoo Partner EMEA 2016 > >
>>>     _______________________________________________ > Mailing-List: >
>>>     https://odoo-community.org/groups/contributors-15
>>>     > > Post to:
>>>     mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org> >
>>>     <mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org>> > Unsubscribe: >
>>>     https://odoo-community.org/groups?unsubscribe
>>>      > > > > >
>>>     ********************************************************************************
>>>     > Ce message et toutes les pièces jointes (ci-après le "message")
>>>     sont établis à > l'intention exclusive de ses destinataires et
>>>     sont confidentiels. Si vous > recevez ce message par erreur,
>>>     merci de le détruire et d'en avertir > immédiatement
>>>     l'expéditeur. Toute utilisation de ce message non conforme à sa >
>>>     destination, toute diffusion ou toute publication, totale ou
>>>     partielle, est > interdite, sauf autorisation expresse.
>>>     L'internet ne permettant pas d'assurer > l'intégrité de ce
>>>     message, SYLEAM décline toute responsabilité au titre de ce >
>>>     message, dans l'hypothèse où il aurait été modifié. > > This
>>>     message and any attachments (the "message") is intended solely
>>>     for the > addressees and is confidential. If you receive this
>>>     message in error, please > delete it and immediately notify the
>>>     sender. Any use not in accord with its > purpose, any
>>>     dissemination or disclosure, either whole or partial, is
>>>     prohibited > except after formal approval. The internet can not
>>>     guarantee the integrity of > this message. SYLEAM cannot
>>>     therefore be liable for the message if modified. >
>>>     ********************************************************************************
>>>     > > _______________________________________________ >
>>>     Mailing-List: > https://odoo-community.org/groups/contributors-15
>>>     > > Post to:
>>>     mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org> >
>>>     <mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org>> > Unsubscribe:
>>>     https://odoo-community.org/groups?unsubscribe
>>>      > > > >
>>>     _______________________________________________ > Mailing-List:
>>>     https://odoo-community.org/groups/contributors-15
>>>      > > Post to:
>>>     mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org> >
>>>     <mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org>> > Unsubscribe:
>>>     https://odoo-community.org/groups?unsubscribe
>>>      > > > >
>>>     _______________________________________________ > Mailing-List:
>>>     https://odoo-community.org/groups/contributors-15
>>>      > > Post to:
>>>     mailto:contributors@odoo-community.org
>>>     <mailto:contributors@odoo-community.org> >
>>>     <mailto:contributors@odoo-community.org
>>> <mailto:contributors@odoo-community.org>> > Unsubscribe: >>> https://odoo-community.org/groups?unsubscribe >>> > > > > >>> _______________________________________________ > Mailing-List: >>> https://odoo-community.org/groups/contributors-15 >>> > Post to: >>> mailto:contributors@odoo-community.org >>> <mailto:contributors@odoo-community.org> > Unsubscribe: >>> https://odoo-community.org/groups?unsubscribe >>> > >> >> _______________________________________________ >> Mailing-List: https://odoo-community.org/groups/contributors-15 >> >> Post to: mailto:contributors@odoo-community.org >> <mailto:contributors@odoo-community.org> >> Unsubscribe: https://odoo-community.org/groups?unsubscribe >> >> >> >> _______________________________________________ >> Mailing-List: https://odoo-community.org/groups/contributors-15 >> Post to: mailto:contributors@odoo-community.org >> Unsubscribe: https://odoo-community.org/groups?unsubscribe >> > > _______________________________________________ > Mailing-List: https://odoo-community.org/groups/contributors-15 > Post to: mailto:contributors@odoo-community.org > Unsubscribe: https://odoo-community.org/groups?unsubscribe >

_______________________________________________
Mailing-List: https://odoo-community.org/groups/contributors-15
Post to: mailto:contributors@odoo-community.org
Unsubscribe: https://odoo-community.org/groups?unsubscribe


Reference