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
[email protected]
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
_______________________________________________
OpenStack-Infra mailing list
[email protected]
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra
