← Back to team overview

fuel-dev team mailing list archive

Re: Process improvements revisited

 

or git review <branch_name> if you are requesting change to another branch
than master


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>

Follow ups

References