Open Stack

On 07/05/2018 12:46 PM, Andrew Grimberg wrote:
> On 07/05/2018 09:13 AM, Jeremy Stanley wrote:
>> On 2018-07-05 09:03:44 -0700 (-0700), Andrew Grimberg wrote:
>>> On 07/04/2018 06:57 PM, Jeremy Stanley wrote:
>> [...]
>>>> For that matter, setting the topic based on the local branch
>>>> name could also get tossed while we're at it, and just keep the
>>>> -t option for directly specifying a change topic when people
>>>> really want to do it at time of upload.
>>>
>>> Personally I would find this a regression. We inform our
>>> communities to use local branches and git-review all the time and
>>> tell them it will take care of setting the topic as long as they
>>> do that. It's an extremely useful feature and I rely upon it
>>> daily! I would hate to have to add an extra flag to my review
>>> pushes.
>>
>> Very helpful feedback, thanks! I'm on the fence about that one
>> simply because the only reason git-review cared to set review topics
>> at all originally was that at the time Gerrit only allowed you to do
>> that when pushing a new commit. They've since separated topic
>> modification out into its own action which can be done from the
>> WebUI or API on an existing change without altering anything else
>> about it. I do find the topic-branch-sets-change-topic behavior sort
>> of unclean from an idempotency standpoint, as `git-review -d`
>> followed by `git review` will alter the topic of your existing
>> change to be the change index number when I'd rather it just left
>> the topic alone.
> 
> Perhaps it shouldn't try setting / resetting the topic if the local
> branch is refs/review/<user>/<change_number> ? That could definitely be
> cleaned up and is a very minor frustration to me, but is very rarely hit
> (that I'm aware of) in our communities.

I hit this all the time - largely because I'm lazy - and it drives me 
batty. I would definitely be in favor of detecting that we're in 
efs/review/<user>/<change_number> and not auto-setting a topic in that case.

>> My bigger concern is that git-review attempts to autodetect possible
>> topic names based on (at this point increasingly outmoded)
>> OpenStack-community-specific commit message footer contents like
>> Implements and Closes-Bug. These I see as a nuisance and
>> codification of OpenStackisms we should cleanse from the codebase.
> 
> Oh, interesting, I didn't know that it tried to do that. We don't have
> those footer semantics in any of our projects at present so it's never
> been something that comes up.
> 
> -Andy-
> 
> 
> 
> _______________________________________________
> OpenStack-Infra mailing list
> OpenStack-Infra at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
>