← Back to team overview

fuel-dev team mailing list archive

Re: Process improvements revisited

 

Dmitry,
thank you for bringing this up in a constructive manner with proposed
changes. With the additional stuff, discussed on the call - let me provide
our agreements.

Before going into every point, we agreed that we are following OpenStack
development model and would like to stick with it as far as we can, and
reuse all the existing tools, such as OpenStack Infra, Gerrit,
github.com/stackforge, launchpad. The closer we are with OpenStack flows,
the easier life for both newcomers into Fuel from OpenStack community and
Fuel developers.

*1. Branch management for maintenance releases*

   - We had 3.2-fixes branch created, it worked good unless minor issues
   with a few forgotten commits which were not proposed to master at the same
   time as they were proposed to master.
   - We will create stable/4.0 branch right before the code freeze
   announcement, which will include all required links / information
   - git whatchanged can not be used to compare branches: it sees merges as
   another commits. Current situation is all required fixes were proposed and
   merged to master from 3.2-fixes - verified by Dmitry Pyzhov (fuel-main,
   fuel-astute), Vladimir Kuklin (fuel-library) and Tatyana Leontovich
   (fuel-ostf)
   - I fully agree that core reviewers must not accept code into stable
   branch unless it is merged into master or patchset has a description why it
   should be only in stable branch. In the long run, we need to see if it is
   possible to configure Jenkins to automatically "-1" such requests to stable
   branches.
   - Let's keep following existing workflows as far as we can go with them:
   https://wiki.openstack.org/wiki/StableBranch
   https://wiki.openstack.org/wiki/GerritJenkinsGit

*2. Management and code review of feature development branches*

   - > reject changes that are too large and/or contain messy revision
   history - all agree on this. All core reviewers - please keep an attention.
   - Code is unreviewed for a long time
   As we moved away from Kanban board - it is harder and harder for leads
   to keep an eye on every patchset. Instead, as we agreed - let's evangelize
   to follow another approach - as a developer, my work is not complete unless
   my code is merged into master and bug / blueprint is closed. So, team
   lead's job here is to control if developers push hard enough to make their
   code reviewed and merged. Dear Team Leads, please take a careful look at
   those patchsets which are not updated for more than a few days - and ping
   developers. Dear Developers, please don't wait to be pinged or punished,
   don't leave your code for abandonment :)
   - > when going through reviews, start with fixes for critical bugs -
   With you here, my ++
   - > Don't merge a review request if there's an older review request that
   can also be merged - With you here too, ++


*3. Bugs triage*
My suggestion is simply to follow:
https://wiki.openstack.org/wiki/Bugs and
https://wiki.openstack.org/wiki/BugTriage.
 I have to admit though that some additional notes are required to make it
easy to set appropriate status, and may be for triaging as well. For
example, if one OSTF test fails because of test itself - it should never be
Critical, but if whole OSTF doesn't work - I believe it should. As we use
OSTF for OpenStack deployment verification, including system tests, we can
easily miss the point when we break something else.
Please do not hesitate to propose additions to the links above specific for
Fuel only.


Some other questions raised:
*WIP branches *should* be maintainable despite on anything*
I believe we should not have such branches despite on anything :) Let's
discuss every situation when it is needed, and see if there is anything we
can do.
I do not want to see such branches, as it will be always hard to merge and
review later. So, if possible, it should be split on several pieces which
could be merged independently, even if many parts are stubbed / not even
enabled (with corresponding comments of course).

*Core reviewers*
As we agreed on the core reviewers/management meeting - Dmitry Borodaenko
is added to the team of core reviewers. Congrats! :)

Bogdan D. - on your question about branches creation, please talk to Dmitry
Pyzhov.

Thanks!


On Wed, Dec 11, 2013 at 5:55 PM, Bogdan Dobrelya <bdobrelia@xxxxxxxxxxxx>wrote:

>  On 12/11/2013 03:42 PM, Vladimir Kuklin wrote:
>
> or git review <branch_name> if you are requesting change to another branch
> than master
>
>  Yes, I already have a forks for most of main fuel repos, I use them for
> storing my WIP branches, because I cannot create any branches at main repo,
> and
> *that* was the 1st subject of discussion for long running R & D activities.
> And the 2nd subject was addressed to "long-lived feature branches with
> many commits and thousands of lines"
>
> (Yet another thing that everyone seems to agree on is that huge
>
> long-lived feature branches with many commits and thousands of lines
> worth of changes are evil and dangerous. Luckily, the move to Gerrit
> will make it hard enough to maintain and merge multi-commit branches,
> and will push people towards committing and merging changes in smaller
> self-sufficient chunks.)
>
> So, I was initially asking to elaborate, what if such WIP branches
> *should* be maintainable despite on anything, and how (what is the best
> practice for now?)
>
>
>
>
> On Wed, Dec 11, 2013 at 5:37 PM, Miroslav Anashkin <manashkin@xxxxxxxxxxxx
> > wrote:
>
>> And do not use push, use `git review` without parameters
>>
>>
>> On Wed, Dec 11, 2013 at 5:36 PM, Miroslav Anashkin <
>> manashkin@xxxxxxxxxxxx> wrote:
>>
>>> Please fork stackforge repo to your account first and use the forked
>>> version
>>>
>>>
>>> On Wed, Dec 11, 2013 at 4:57 PM, Bogdan Dobrelya <bdobrelia@xxxxxxxxxxxx
>>> > wrote:
>>>
>>>>  On 12/11/2013 01:35 PM, Oleg Gelbukh wrote:
>>>>
>>>> Looks like fuel.config already have this configuration actually:
>>>>
>>>>
>>>> https://git.openstack.org/cgit/openstack-infra/config/tree/modules/openstack_project/files/gerrit/acls/stackforge/fuel.config#n5
>>>>
>>>>  Did you check if you actually can create a branch?
>>>>
>>>>  Yes, I've tried git push -u origin foo_branch and I've got
>>>> ERROR: Permission to stackforge/fuel-web(library).git denied to
>>>> bogdando.
>>>>
>>>>
>>>>
>>>>  --
>>>> Best regards,
>>>> Oleg Gelbukh
>>>>
>>>>
>>>>  On Wed, Dec 11, 2013 at 2:36 PM, Bogdan Dobrelya <
>>>> bdobrelia@xxxxxxxxxxxx> wrote:
>>>>
>>>>>  On 12/11/2013 11:52 AM, Oleg Gelbukh wrote:
>>>>>
>>>>> Bogdan,
>>>>>
>>>>>  In OpenStack CI, that is configured in openstack-ci/config
>>>>> repository. You have to add certain lines to gerrit access lists
>>>>> configuration
>>>>> (modules/openstack_project/files/gerrit/acls/stackforge/fuel.config) for
>>>>> your project there:
>>>>>
>>>>>  [access refs/*]
>>>>> create = group <your-project-name>-core
>>>>>
>>>>>  or something like that. Please, ask at openstack-infra ML or
>>>>> #openstack-infra for more precise advice.
>>>>>
>>>>>
>>>>> Looks like the openstack-ci/config repo is about projects management,
>>>>> not its WIP branches - the only reference for branch named
>>>>> "feature/ec"  (mentioned in
>>>>> http://lists.openstack.org/pipermail/openstack-dev/2013-July/012102.html)
>>>>> I've found is:
>>>>> ./modules/gerritbot/files/gerritbot_channel_config.yaml:
>>>>> openstack-swift:
>>>>>     events:
>>>>>       - patchset-created
>>>>>       - change-merged
>>>>>       - x-vrif-minus-2
>>>>>     projects:
>>>>>       - openstack/swift
>>>>>       - openstack/swift-bench
>>>>>       - openstack/python-swiftclient
>>>>>     branches:
>>>>>       - master
>>>>>       - feature/ec
>>>>>
>>>>> So, it is still unclear how to create new branches for review...
>>>>> I will consult with #openstack-infra anyway, thank you.
>>>>> I believe we should have this question clearly elaborated for our R&D
>>>>> teams.
>>>>>
>>>>>
>>>>>
>>>>>  --
>>>>> Best regards,
>>>>> Oleg Gelbukh
>>>>>
>>>>>
>>>>> On Wed, Dec 11, 2013 at 1:42 PM, Bogdan Dobrelya <
>>>>> bdobrelia@xxxxxxxxxxxx> wrote:
>>>>>
>>>>>>  On 12/11/2013 11:06 AM, Oleg Gelbukh wrote:
>>>>>>
>>>>>> Bogdan,
>>>>>>
>>>>>>  You might be interested in the approach taken by Swift team for
>>>>>> long-term development effort of erasure coding storage option:
>>>>>>
>>>>>> http://lists.openstack.org/pipermail/openstack-dev/2013-July/012102.html
>>>>>>
>>>>>>  Thank you, the approach is good indeed. Do we have a rights or
>>>>>> work-flow for creating WIP branches of our main repos?
>>>>>>
>>>>>>
>>>>>>  --
>>>>>> Best regards,
>>>>>> Oleg Gelbukh
>>>>>>
>>>>>>
>>>>>> On Wed, Dec 11, 2013 at 1:02 PM, Bogdan Dobrelya <
>>>>>> bdobrelia@xxxxxxxxxxxx> wrote:
>>>>>>
>>>>>>> Hello.
>>>>>>>
>>>>>>>
>>>>>>> On 12/10/2013 09:14 PM, Dmitry Borodaenko wrote:
>>>>>>>
>>>>>>>> All,
>>>>>>>>
>>>>>>>> We still have a few pain points left in our development process
>>>>>>>> that I
>>>>>>>> think are easy to fix with a bunch of simple rules. I think
>>>>>>>> releasing
>>>>>>>> 4.0 will be less painful if we try to address these.
>>>>>>>>
>>>>>>>> 1. Branch management for maintenance releases
>>>>>>>>
>>>>>>>> We already had this discussion during 3.2.1 release cycle, and
>>>>>>>> agreed
>>>>>>>> to follow the approach that is in line with what OpenStack and most
>>>>>>>> other free software projects are following. Still, I think we should
>>>>>>>> do better at actually following the process we agreed to.
>>>>>>>>
>>>>>>>> To see how good we were at following it for 3.2.1, open two terminal
>>>>>>>> windows and run:
>>>>>>>>
>>>>>>>> git whatchanged 3.2..3.2-fixes
>>>>>>>> git whatchanged 3.2..master
>>>>>>>>
>>>>>>>> and for each commit in 3.2-fixes, try to find a matching fix in
>>>>>>>> master. Last time I checked there were still many cases where
>>>>>>>> bugfixes
>>>>>>>> were merged to 3.2-fixes before (or even without) merging them to
>>>>>>>> master. Did anyone actually check that we're not missing any
>>>>>>>> important
>>>>>>>> fixes from 3.2.1 in 4.0?
>>>>>>>>
>>>>>>>> We should create a new stable/4.0 branch as soon as 4.0 code freeze
>>>>>>>> is
>>>>>>>> announced (ideally, the announcement itself should direct committers
>>>>>>>> to the new branch). Reviewers should REJECT all commits to
>>>>>>>> stable/4.0
>>>>>>>> that have not been merged into master, unless a justification is
>>>>>>>> provided in the COMMIT MESSAGE.
>>>>>>>>
>>>>>>>  Can Jenkins help us by -1 such patches?
>>>>>>> I.e. Jenkins could put -1 to any patch targeted for non-master,
>>>>>>> unless its commits were found in master.
>>>>>>>
>>>>>>>
>>>>>>>> 2. Management and code review of feature development branches
>>>>>>>>
>>>>>>>> Yet another thing that everyone seems to agree on is that huge
>>>>>>>> long-lived feature branches with many commits and thousands of lines
>>>>>>>> worth of changes are evil and dangerous. Luckily, the move to Gerrit
>>>>>>>> will make it hard enough to maintain and merge multi-commit
>>>>>>>> branches,
>>>>>>>> and will push people towards committing and merging changes in
>>>>>>>> smaller
>>>>>>>> self-sufficient chunks.
>>>>>>>>
>>>>>>>  That should we do for long running researches, such as HA
>>>>>>> improvements
>>>>>>> (started at 3.1, targeted to 4.1 only), or torrent based
>>>>>>> provisioning?
>>>>>>> Should we melt down hundreds of commits into a single patch in WIP
>>>>>>> branch,
>>>>>>> before submitting new feature to review?
>>>>>>>
>>>>>>>
>>>>>>>> A recent negative example is the fuel-library pull request #911 that
>>>>>>>> has merged 104 duplicate commits from ancient alternative history
>>>>>>>> into
>>>>>>>> master, instead of simply rebasing a single commit. The only way to
>>>>>>>> prevent something like this from happening is to summarily reject
>>>>>>>> changes that are too large and/or contain messy revision history.
>>>>>>>>
>>>>>>>  Jenkins could come to help here as well. E.g. -1, if any commit in
>>>>>>> PR are
>>>>>>> already present in target branch's history.
>>>>>>>
>>>>>>>
>>>>>>>> The other side of the same problem is holding back small reasonable
>>>>>>>> changes for too long, placing unnecessary burden on authors to keep
>>>>>>>> rebasing their change on top of other changes that got merged
>>>>>>>> earlier.
>>>>>>>>
>>>>>>>> For example, my own fuel-docs pull request #67 sat unreviewed for a
>>>>>>>> week only to be obsoleted by the move of the repo to StackForge
>>>>>>>> (after
>>>>>>>> being obsoleted couple more times by changes that were merged ahead
>>>>>>>> of
>>>>>>>> it). I suspect most other developers had similar experiences. On top
>>>>>>>> of obvious frustration, holding a change back tempts the author to
>>>>>>>> keep piling changes onto the same request instead of creating a new
>>>>>>>> review request on top of updated master for their next set of
>>>>>>>> changes.
>>>>>>>> To use the same example, most of the third commit on #67 should
>>>>>>>> really
>>>>>>>> have been a separate pull request.
>>>>>>>>
>>>>>>>> The fix is once again rather obvious: when going through reviews,
>>>>>>>> start with fixes for critical bugs, then go through remaining
>>>>>>>> reviews
>>>>>>>> starting with the least recently updated ones. Don't merge a review
>>>>>>>> request if there's an older review request that can also be merged.
>>>>>>>>
>>>>>>>> I'm using this link to see all our outstanding review requests:
>>>>>>>> https://review.openstack.org/#/q/status:open+project
>>>>>>>> :^stackforge/fuel-.*,n,z
>>>>>>>>
>>>>>>>> Right now I see that there are review requests that have +1 from CI
>>>>>>>> and from reviewers (meaning they can be merged) sitting unchanged
>>>>>>>> since Nov 25, and a few unreviewed requests going as far back as Nov
>>>>>>>> 3. We shouldn't have a request sit untouched by an approver for more
>>>>>>>> than a week, let alone a month. If there's a any reason you don't
>>>>>>>> want
>>>>>>>> to merge it, give it -1 and explain. Otherwise, there's no reason
>>>>>>>> not
>>>>>>>> to give it +2. If you have time to review and merge a newer request,
>>>>>>>> you have time for that older one, too.
>>>>>>>>
>>>>>>>> 3. Bugs triage
>>>>>>>>
>>>>>>>> Moving our bug tracking to public launchpad was an important step
>>>>>>>> towards opening up our development process, now we should improve
>>>>>>>> visibility of our bugs triage and release management processes. In
>>>>>>>> addition to announcing target release dates, we should also have
>>>>>>>> well
>>>>>>>> defined release criteria (for example, no critical bugs affecting
>>>>>>>> the
>>>>>>>> upcoming release, no more than 5 bugs with high importantce, etc.),
>>>>>>>> and documented rules on how to set importance of a bug. We don't
>>>>>>>> have
>>>>>>>> to be rigid and beaurocratic about it, but having documented
>>>>>>>> criteria
>>>>>>>> will help all participants of the process prioritize their own work
>>>>>>>> and understand how it fits into the state of the whole project. It
>>>>>>>> will also help avoid situations like missing an important bugfix in
>>>>>>>> a
>>>>>>>> release, by forcing us to review priorities of all open bugs before
>>>>>>>> announcing a release.
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>>  Best regards,
>>>>>>> Bogdan Dobrelya,
>>>>>>> Researcher TechLead, Mirantis, Inc.
>>>>>>> +38 (066) 051 07 53
>>>>>>> Skype bogdando_at_yahoo.com
>>>>>>> 38, Lenina ave.
>>>>>>> Kharkov, Ukraine
>>>>>>> www.mirantis.com
>>>>>>> www.mirantis.ru
>>>>>>> bdobrelia@xxxxxxxxxxxx
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Mailing list: https://launchpad.net/~fuel-dev
>>>>>>> Post to     : fuel-dev@xxxxxxxxxxxxxxxxxxx
>>>>>>> Unsubscribe : https://launchpad.net/~fuel-dev
>>>>>>> More help   : https://help.launchpad.net/ListHelp
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards,
>>>>>> Bogdan Dobrelya,
>>>>>> Researcher TechLead, Mirantis, Inc.
>>>>>> +38 (066) 051 07 53
>>>>>> Skype bogdando_at_yahoo.com
>>>>>> 38, Lenina ave.
>>>>>> Kharkov, Ukrainewww.mirantis.comwww.mirantis.rubdobrelia@xxxxxxxxxxxx
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Best regards,
>>>>> Bogdan Dobrelya,
>>>>> Researcher TechLead, Mirantis, Inc.
>>>>> +38 (066) 051 07 53
>>>>> Skype bogdando_at_yahoo.com
>>>>> 38, Lenina ave.
>>>>> Kharkov, Ukrainewww.mirantis.comwww.mirantis.rubdobrelia@xxxxxxxxxxxx
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards,
>>>> Bogdan Dobrelya,
>>>> Researcher TechLead, Mirantis, Inc.
>>>> +38 (066) 051 07 53
>>>> Skype bogdando_at_yahoo.com
>>>> 38, Lenina ave.
>>>> Kharkov, Ukrainewww.mirantis.comwww.mirantis.rubdobrelia@xxxxxxxxxxxx
>>>>
>>>>
>>>> --
>>>> Mailing list: https://launchpad.net/~fuel-dev
>>>> Post to     : fuel-dev@xxxxxxxxxxxxxxxxxxx
>>>> Unsubscribe : https://launchpad.net/~fuel-dev
>>>> More help   : https://help.launchpad.net/ListHelp
>>>>
>>>>
>>>
>>>
>>> --
>>>
>>> *Kind Regards*
>>>
>>> * Miroslav Anashkin **L2 support engineer**,*
>>> *Mirantis Inc.*
>>> *+7(495)640-4944 (office receptionist)*
>>> *+1(650)587-5200 (office receptionist, call from US)*
>>> *35b, Bld. 3, Vorontsovskaya St.*
>>> *Moscow**, Russia, 109147.*
>>>
>>> www.mirantis.com
>>>
>>> manashkin@xxxxxxxxxxxx
>>>
>>>
>>
>>
>> --
>>
>> *Kind Regards*
>>
>> * Miroslav Anashkin **L2 support engineer**,*
>> *Mirantis Inc.*
>> *+7(495)640-4944 (office receptionist)*
>> *+1(650)587-5200 (office receptionist, call from US)*
>> *35b, Bld. 3, Vorontsovskaya St.*
>> *Moscow**, Russia, 109147.*
>>
>> www.mirantis.com
>>
>> manashkin@xxxxxxxxxxxx
>>
>>
>> --
>> Mailing list: https://launchpad.net/~fuel-dev
>> Post to     : fuel-dev@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~fuel-dev
>> More help   : https://help.launchpad.net/ListHelp
>>
>>
>
>
> --
> Yours Faithfully,
> Vladimir Kuklin,
> Senior Deployment Engineer,
> Mirantis, Inc.
> +7 (495) 640-49-04
> +7 (926) 702-39-68
> Skype kuklinvv
> 45bk3, Vorontsovskaya Str.
> Moscow, Russia,
> www.mirantis.com <http://www.mirantis.ru/>
> www.mirantis.ru
> vkuklin@xxxxxxxxxxxx <nveschikova@xxxxxxxxxxxx>
>
>
>
> --
> Best regards,
> Bogdan Dobrelya,
> Researcher TechLead, Mirantis, Inc.
> +38 (066) 051 07 53
> Skype bogdando_at_yahoo.com
> 38, Lenina ave.
> Kharkov, Ukrainewww.mirantis.comwww.mirantis.rubdobrelia@xxxxxxxxxxxx
>
>
> --
> Mailing list: https://launchpad.net/~fuel-dev
> Post to     : fuel-dev@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~fuel-dev
> More help   : https://help.launchpad.net/ListHelp
>
>


-- 
Mike Scherbakov
#mihgen

Follow ups

References