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. > 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- -- Andrew J Grimberg Lead, IT Release Engineering The Linux Foundation
signature.asc
Description: OpenPGP digital signature
_______________________________________________ OpenStack-Infra mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
