Re: [ANNOUNCE] git-series: track changes to a patch series over time
On Thu, Jul 28, 2016 at 11:40:55PM -0700, Josh Triplett wrote: [snip] > > I'd welcome any feedback, whether on the interface and workflow, the > internals and collaboration, ideas on presenting diffs of patch series, > or anything else. > This looks awesome! I've been working on some similar stuff for a while also.[1][2] I'm particularly interested in trying to establish a standard for storing review data in git. I've got a prototype for doing that[3], and an example tool that uses it[4]. The tool is still incomplete/buggy though. There seem to be a number of us trying to solve this in our different ways, it would be great to coordinate our efforts. The prototype library I have is partly the result of some discussion and work with the Gerrit folks, since they were thinking about this problem before I even started writing git-candidate, and solved it with Notedb.[5] Let me know if you'd like to work together on this, I've been considering taking the perl-notedb prototype and writing a C library for it with bindings for other languages (i.e. Rust). [1]: http://www.mail-archive.com/git%40vger.kernel.org/msg79461.html [2]: http://www.mail-archive.com/git%40vger.kernel.org/msg80972.html [3]: https://bitbucket.org/richardipsum/perl-notedb [4]: https://bitbucket.org/richardipsum/git-candidate [5]: https://storage.googleapis.com/gerrit-talks/summit/2015/NoteDB.pdf
Re: [ANNOUNCE] git-series: track changes to a patch series over time
On Fri, Jul 29, 2016 at 04:04:26AM -0700, Josh Triplett wrote: [snip] > > These definitely seem like a family of related problems. I'd like to > use git-series as a format for storing iterations on things like GitHub > pull-requests or Gerrit patch versions (in the latter case, overcoming > Gerrit's limitations on only handling one patch at a time). Integrating > reviews with that seems helpful. Worth noting here that Gerrit's one patch per change format isn't intrinsic to Notedb, since we just need to track the sha we want to merge and optionally the branch we intend to merge into. > > > The prototype library I have is partly the result of some discussion and > > work > > with the Gerrit folks, since they were thinking about this problem > > before I even started writing git-candidate, and solved it with Notedb.[5] > > > > Let me know if you'd like to work together on this, > > I'd love to. > > I'll be presenting git-series at LinuxCon North America; will you be > there by any chance? If not, perhaps we could meet by IRC or some other > medium and talk about this family of problems. Cool :) I didn't plan to be at LinuxCon North America, but I can certainly send contact details out of band. > > I hope to use git notes with git-series in the future, by putting > another gitlink under the git-series for notes related to the series. > I'd intended that for more persistent notes; putting them in the series > solves some of the problems related to notes refs, pushing/pulling, and > collaboration. Using notes for review comments makes sense as well, > whether in a series or in a separate ref. Sounds interesting, can you explain how this works in more detail? I ended up solving the push/pull issue with a custom merge driver that effectively runs the Notedb parser on each side of the merge and emits the union of the two sets of change notes. > > > I've been considering taking the perl-notedb prototype and writing > > a C library for it with bindings for other languages (i.e. Rust). > > A C library based on libgit2 seems like a good idea; ideally the > bindings could interoperate with git2-rs. (Alternatively, Rust can > *export* a C interface, so you could write directly with git2-rs. :) ) Certainly a fair alternative, though it may arguably be safer to write the C and export to other languages, as cool as Rust looks it's not established the way C is, so may be a slightly riskier foundation, in my view. And ofcourse in C we have native access to libgit2. > > One of the items on my long-term TODO list is a completely federated > GitHub; I've been looking at other aspects of that, but federated > reviews/comments/etc seem critical to that as well. > I agree.
Re: [ANNOUNCE] git-series: track changes to a patch series over time
On Fri, Jul 29, 2016 at 09:59:08AM -0700, Stefan Beller wrote: > On Fri, Jul 29, 2016 at 5:44 AM, Richard Ipsum > wrote: > >> > >> These definitely seem like a family of related problems. I'd like to > >> use git-series as a format for storing iterations on things like GitHub > >> pull-requests or Gerrit patch versions (in the latter case, overcoming > >> Gerrit's limitations on only handling one patch at a time). Integrating > >> reviews with that seems helpful. > > > > Worth noting here that Gerrit's one patch per change format isn't > > intrinsic to Notedb, since we just need to track the sha we want > > to merge and optionally the branch we intend to merge into. > > Note that Gerrit started to lose the "one patch at a time" notion. > It is possible to at least submit multiple changes coupled together > (even across project boundaries) via the topic. Some sort of cover > letter is missing though, that could be used e.g. for the merge commit. Potentially my misuse of the format but git-candidate puts the cover letter into the body of the commit message before the footers begin, for each new patchset added to the change. This has the advantage that you can track each version of the cover letter, since there's one per patchset.
Re: [ANNOUNCE] git-series: track changes to a patch series over time
On Fri, Jul 29, 2016 at 06:00:55AM -0700, Josh Triplett wrote: > On Fri, Jul 29, 2016 at 01:44:44PM +0100, Richard Ipsum wrote: > > On Fri, Jul 29, 2016 at 04:04:26AM -0700, Josh Triplett wrote: > > > I hope to use git notes with git-series in the future, by putting > > > another gitlink under the git-series for notes related to the series. > > > I'd intended that for more persistent notes; putting them in the series > > > solves some of the problems related to notes refs, pushing/pulling, and > > > collaboration. Using notes for review comments makes sense as well, > > > whether in a series or in a separate ref. > > > > Sounds interesting, can you explain how this works in more detail? > > The tree within a git-series commit includes a blob "cover" for the > cover letter, a gitlink "base" for the base commit, and a gitlink > "series" for the top of the series. I could add a gitlink "notes", > which acts like a notes ref; then, each version of the series would have > its own notes ref. As with the series, git-series would track the > "history of history"; since git-notes themselves use git history to > store a set of notes, git-series would store the history of the notes. > So if you add, remove, or change a note, git-series would track that as > a change to the notes ref. If you merge/rebase/etc the notes ref to > merge notes, git-series would track that too. A different series would > have a different set of notes, so you wouldn't be limited to > one notes ref per repository. > > This doesn't solve the problem of merging notes, but it *does* mean you > have a full history of the changes to notes, not just the notes > themselves. > > Something similar might work for the Gerrit notesdb. > Okay I think there is a misunderstanding, Notedb is based on notes, but they're not used in the same way as git-notes, an example will help explain what I mean, For a candidate 'update_readme' we store the change/candidate/whatever metadata at refs/candidates/heads/up/update_readme/meta which is analogous to Gerrit's notedb refs which uses something like refs/changes/34/1234/meta, the prototype library I've written supports both forms and allows for some flexibility in the naming of the prefix of the former type of ref (so you may use refs/series/heads/up/update_readme/meta for example). So the output of, git log -p refs/candidates/heads/up/update_readme/meta gives commit 38d0c182a46dc5a0f5d04ea0890e278b8e7a6eb6 Author: Richard Ipsum Date: Sun Jul 24 16:59:16 2016 +0100 Metadata update Patch-set: 1 Status: merged commit f45a396a156e121f923321e7530e74746e10bdb8 Author: Richard Ipsum Date: Sun Jul 24 16:50:13 2016 +0100 Vote on patch set 1 Label: CodeReview=+1 Patch-set: 1 commit b74eb15c1847d3bb28618c738c8ebc3412b6935a Author: Richard Ipsum Date: Sun Jul 24 16:48:11 2016 +0100 Update our README to reflect reality BranchCommit; 59c46c9fa03725308779841f95ad71e7ccdb919c Branch: master Commit: 761d8da03a10b63b0b1e3cf97ffd7ececb09e3d6 Patch-set: 1 Status: new Subject: update_readme This Notedb history is the result of the following git-candidate invocations git candidate create update_readme -m "Update our README to reflect reality" git candidate vote +1 (use whatever git commands you like to merge the change) git candidate close update_readme Basically any change made to a change in Notedb is recorded in a git history. The format is explained in some more detail here[1]. [1]: https://storage.googleapis.com/gerrit-talks/summit/2015/NoteDB.pdf
Re: [ANNOUNCE] git-series: track changes to a patch series over time
On Mon, Aug 01, 2016 at 01:59:29AM -0700, Josh Triplett wrote: > On Mon, Aug 01, 2016 at 07:55:54AM +, Eric Wong wrote: [snip] > > > > I'm not convinced another format/standard is needed besides the > > email workflow we already use for git and kernel development. > > Not all projects use a patches-by-email workflow, or want to. To the > extent that tools and projects use some other workflow, standardizing > the format they use to store patch reviews (including per-line > annotations, approvals, test results, etc) seems preferable to having > each tool use its own custom format. I concur, for better or for worse many projects have abandoned mailing lists in favour of github, gerrit, gitlab and the like. The problem being, with the exception of gerrit, most of these tools store review data in sql databases, which is bad for obvious reasons. > > > I also see the reliance on an after-the-fact search engine > > (which can be tuned/replaced) as philosophically inline with > > what git does, too, such as not having rename tracking and > > doing delayed deltafication. > > Storing review data in git doesn't mean it needs to end up in the > history of the project itself; it can use after-the-fact annotations on > a commit. Exactly.
Re: [ANNOUNCE] git-series: track changes to a patch series over time
On Thu, Aug 04, 2016 at 12:40:58PM -1000, Josh Triplett wrote: > On Wed, Aug 03, 2016 at 08:12:02PM +0100, Richard Ipsum wrote: > > On Thu, Jul 28, 2016 at 11:40:55PM -0700, Josh Triplett wrote: > > > I'd welcome any feedback, whether on the interface and workflow, the > > > internals and collaboration, ideas on presenting diffs of patch series, > > > or anything else. > > > > One other nice thing I've noticed about this tool is the > > way series behave like regular git branches: I specify the name > > of the series and from then on all other commands act on that > > series until told otherwise. > > Thanks; I spent a while thinking about that part of the workflow. I > save the current series as a symbolic ref SHEAD, and everything operates > on SHEAD. (I should probably add support for running things like "git > series log" or "git series format" on a different series, because right > now "until told otherwise" doesn't include a way to tell it otherwise.) Apologies for this delayed response, I needed time to gather my thoughts, and also to fix the perl libgit2 binding to allow me to use your symbolic ref suggestion. :p Though it turns out that libgit2 doesn't currently allow me to write arbitrary data to a symbolic ref as git-symbolic-ref(1) will, so this still needs to be fixed somehow. > > One fun detail that took a couple of iterations to get right: I keep > separate "staged" and "working" versions per-series, so even with > outstanding changes to the cover letter, base, or series, you can always > detach or checkout another series without losing anything. If you > switch back, all your staged and unstaged changes will remain staged and > unstaged where you left them. That solves the "checkout a different > series with modifications to the current series" case. Cool > > > git-appraise looks as though it might also have this behaviour. > > I think it's a nice way to do it, since you don't generally > > perform more than one review simultaneously. So I may well > > use this idea in git-candidate if it's okay. :) > > By all means. For a review tool like git-candidate, it seems like you'd > want even more contextual information, to make it easier to specify > things like "comment on file F line L". For instance, what if you > spawned the diff to review in an editor, with plenty of extra context > and a file extension that'll cause most editors to recognize it as a > patch (and specifically a git-candidate patch to allow specialized > editor modes), and told people to add their comments after the line they > applied to? When the editor exits successfully, you can scan the file, > detect the added lines, and save those as comments. You could figure > out the appropriate line by looking for the diff hunk headers and > counting line numbers. I really like this idea, the current interface for commenting is a little tedious I find. > > If you use a format-patch diff that includes the headers and commit > message, you could also support commenting on those in the same way. > Does the notedb format support commenting on those? Comments in notedb are just a git note keyed on the sha of the commit being commented on, I'm not certain what advantage a format-patch diff provides in this case? I've been closely following the 'patch submission process' thread, and given the discussion there I'm having doubts over the value of comments in git-candidate vs the mailing list. It seems to me that git-candidate has many of the disadvantages of Github/Gitlab when it comes to comments, for example, there is no threading. Also the system would be less open than the mailing list, since, as it stands currently you would require push access to the repository to comment on anything. It may be worth reflecting that one reason some organisations have switched away from mailing list reviews to Github/Gitlab is that they provide patch tracking, where the mailing list provides none, so patches there can be 'lost'. So instead of trying to reimplement an entire Gerrit/Github/Gitlab ui on the commandline, I wonder whether it would be sufficient to add the minimum functionality necessary to provide git with native patch tracking, and leave comments for the mailing list. Ofcourse this is exactly what git-series seems to do, so in some sense I may be advocating dropping my own work in favour of improving git-series. On the other hand, relying on the mailing list means that some of the history of a series is left outside of the repository which is anathema to the goal of git based/stored review, not least because mail archives are centralised. (which can obviously be problematic (as we
Re: [ANNOUNCE] git-series: track changes to a patch series over time
On Thu, Jul 28, 2016 at 11:40:55PM -0700, Josh Triplett wrote: > > I'd welcome any feedback, whether on the interface and workflow, the > internals and collaboration, ideas on presenting diffs of patch series, > or anything else. > One other nice thing I've noticed about this tool is the way series behave like regular git branches: I specify the name of the series and from then on all other commands act on that series until told otherwise. git-appraise looks as though it might also have this behaviour. I think it's a nice way to do it, since you don't generally perform more than one review simultaneously. So I may well use this idea in git-candidate if it's okay. :) I haven't found time to use the tool to do any serious review yet, but I'll try and post some more feedback when I do. Hope this helps, Richard
