On 2018-07-04 22:32:53 +0100 (+0100), Darragh Bailey wrote: [...] > Based on the comments at > https://git.openstack.org/cgit/openstack-infra/git-review/tree/CONTRIBUTING.rst#n5, > git-review is considered feature complete, and as a consequence it > seems that reviewers have mostly moved onto other projects so it > can take quite some time to get reviews. Perfectly understandable, > everyone can only do so much and needs to pick something(s) to > prioritise. However this is such a useful tool for working with > Gerrit from the command line it seems to be the defacto git > subcommand for interfacing with Gerrit that it seems a shame to > limit it.
At one time git-review had started to suffer from scope creep and was getting more random proposals for new features than actual stability improvements. Its test coverage was, for quite a long while, also sub-par so that some of the feature additions which did get accepted introduced regressions that went unnoticed sometimes for weeks or months before we'd discover they needed reverting. The original authors intended for git-review to be primarily focused on bootstrapping Gerrit connectivity from a cloned Git repository as well as simplifying the basic Git commands for retrieving changes from and pushing changes to a Gerrit. That update to the CONTRIBUTING.rst file was intended to put the brakes on future scope creep, especially in cases where an added feature would work just as well as its own separate git subcommand. > While I think there are a number of current reviews that would be > beneficial to git-review, as well as some pieces that don't appear > to be there currently, I'm reluctant to invest much time as it > seems unlikely enhancements would be accepted due to the current > state of feature complete. Instead of putting together various > changes to see if they might be reviewed and accepted, hoping a > chat about what paths might be available could save a bit of time. I've tried to go in and approve changes from time to time, but in all honesty the negativity I've received in the past when attempting to push back on feature additions has caused me to deprioritize reviewing more changes for it. I should probably just buck up and go in with a (very polite) machete anyway. > There are a couple of things that I would like to work towards: > > * Change the tests to use a single gerrit with separate projects > instead of separate instances (faster testing) This seems reasonable if it doesn't introduce new races or odd test interdependencies from the reduced fixture isolation. I really have never been fond of the integration-testing-only model we ended up with though. I originally recommended lower-level unit testing with mocks for the Git and SSH interactions, but the one volunteer we got to implement a testsuite chose to automate Gerrit installation so it is what it is at the moment. > * Allow the tests to run against multiple versions of Gerrit (ensure > compatibility) This seems reasonable. We should have been bumping the Gerrit versions in the tests and/or running more jobs for different releases of it but the way version selection was implemented would need a bit of an overhaul to accommodate that. > * Fix and land many of the changes making it easier to download > changes, list changes ordered with their dependencies, stashing > when downloading, etc 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. > * Have git-review auto configure refs/notes/review (assuming it's > available) for fetching on setup (I find it very handy and I'm > always forgetting to do this) I could see this being in scope, as it fits with the Gerrit connectivity bootstrapping mission. 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. > And potentially controversially; support other workflows and > options outside of the OpenStack workflow. Although maybe not > directly, and still keeping the OpenStack one as the default. 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. One that I'm aware of is the default topic mangling based on commit message parsing, which I've been wanting to eradicate for a while since Gerrit now makes altering topics possible without needing to push a new commit. 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. > I think there are a couple of ways that could be achieved, but I > can't see any of them working well without a decent amount of > refactoring. > > * Have git-review provide the APIs so that someone may define a > git-review-<name> that can add their workflow > > * Add support for additional behaviour to be defined with > refs/meta/config of projects > > * Allow extensions to be installed that allow additional options > to be added to the git-review CLI and config file > > That last one might require being able to specify the additional > required plugins to be listed in .gitreview, and providing the > documentation might be trickier? > > Basically make it easier to add custom behaviour without it being > builtin to git-review, and without needing to reimplement a whole > load of functionality elsewhere. But I'm pretty sure that all > requires a substantial rewrite. I'd need some concrete use case examples. From my perspective, git-review is already a plugin (by way of being a git subcommand) so adding plugins to the plugin seems like a layer violation. The examples I've seen in the past for adding new behaviors were things which made more sense to me as new git subcommands. For a counterexample, James Blair created git-restack not too long ago... it could have been implemented as a git-review option, but was sanely made to be its own distinct git subcommand instead. > Thoughts? Is it worth putting a plan together around some of the > initial changes? And then revisiting what would be needed to allow > extensions around other workflows? I'm all for plans to improve git-review's stability, test coverage and, most of all, simplicity. Thanks for raising the topic! -- 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