On 2018-07-06 23:54:37 +0100 (+0100), Darragh Bailey wrote: > On Thu, 5 Jul 2018, 02:57 Jeremy Stanley, <fu...@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
signature.asc
Description: PGP signature
_______________________________________________ OpenStack-Infra mailing list OpenStack-Infra@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-infra