Open Stack

On 2018-07-06 23:54:37 +0100 (+0100), Darragh Bailey wrote:
> On Thu, 5 Jul 2018, 02:57 Jeremy Stanley, <fungi at yuggoth.org> wrote:
[...]
> > The change listing feature really seems increasingly out of place to
> > me, and most of the "fixes" I saw related to it were about
> > supporting more and more of Gerrit's query language and terminal
> > formatting. If we deprecated/removed that and recommended
> > interacting directly with Gerrit or alternative utilities for change
> > searches (there are a lot more options for this than there were back
> > when git-review was first written) all of those would become
> > unnecessary and the code would be simplified at the same time.
> 
> That's interesting, I'd consider the ability to query for what is
> available for review a step before downloading a change for
> review, and understanding that it might be bringing multiple
> chances down that aren't merged useful.
> 
> If this was moved out of git-review, I suspect it might still need
> to know a bit about git-review and be able to use some of its
> configuration.
[...]

I guess it comes down to whether we think "finding changes in
Gerrit" is really within scope. For me it hasn't been for a very
long time, as there are other tools far better at this. If I use
git-review to retrieve a change I pretty much already know the
change number (either because I had it pulled up in gertty or the
Gerrit WebUI or saw it mentioned by an IRC bot or in an E-mail
update notification or because someone directly asked me about it).

> > I too find the notes refs handy but have a global configuration
> > in my ~/.gitconfig which seems to do the trick already so I'm
> > curious to find out how git-review might improve on that.
> 
> I didn't think that this could be done for the 'origin' remote and
> would be ignored by fit for other projects where it doesn't exist?
> Or are you using the 'gerrit' remote?

Yes, I do it globally for all remotes named "origin". At least the
git versions I use and the non-Gerrit origins with which I interact
don't seem to get confused when there's no refs/notes/review in a
repo. And for Gerrit-hosted repos, Gerrit replication includes notes
refs even if I'm cloning from the replica rather than directly from
Gerrit. If there are corner cases this breaks, I'd appreciate
knowing about them so I can help with a better implementation, but
so far it's worked out great for me.

> But to the main advantage is that it opens this up to many people
> that might not have been aware it exists. And also opens up to
> asking the user if they'd like the review notes displayed along
> with the logs by default for this repo by setting core.notesRef.
[...]

Yes, this is one of the reasons I consider it to still be in scope.
It's a bootstrapping situation.

> > I'd love to know what about git-review is focused on OpenStack's
> > workflow. We tried to make it as generic as possible. If there
> > are any OpenStack-specific features still lingering in there, we
> > should see about ripping them out as soon as is feasible.
[...]
> There may be others, I recall this coming up before around being
> able to set review scores for labels at the same time as uploading
> the change. Think it was around the 'Workflow' label. Maybe this
> is a case for a different command, but it seems likely to break
> the flow doe anyone using git-review to submit. However the labels
> for each project are customizable so it seems likely this would
> need the correct set to be worked out at setup time if included.
[...]

Some of that situation hails from the dark ages when the OpenStack
community ran a fork of Gerrit with a "work in progress" feature we
were unable to get upstreamed. There was pressure to get git-review
to support it (a -w flag landed in the codebase at some point for
this) but that was a very clear OpenStackism and even long before
the OpenStack community switched back to mainline Gerrit and started
using a "Workflow" label to indicate work in progress status (among
other things) the short-lived -w option was ripped out of git-review
in a mass revert to put the brakes on the runaway train it was
becoming: https://review.openstack.org/13890

As mentioned in the more recent review(s), setting arbitrary labels
at upload might be acceptable if we can come up with a clean
solution for that, but encoding OpenStack community assumptions like
"Workflow=-1 means work-in-progress" must be avoided. Problem is
that each deployment's (even each repository's/ACL's) use of
non-default labels would need some semantic policy description
language in the .gitreview file unless we really do want it to just
be a insert-arbitrary-label-votes-here sort of thing.

An interesting aside, latest versions of Gerrit implement a "work in
progress" feature to replace the terrible "draft" functionality, so
the OpenStack community's use of Workflow=-1 to signal that may
cease to be relevant soon after the next review.openstack.org
upgrade.
-- 
Jeremy Stanley
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 963 bytes
Desc: not available
URL: <http://lists.openstack.org/pipermail/openstack-infra/attachments/20180706/3b698686/attachment.sig>