Re: Patches for git-push --confirm and --show-subjects
- From
Owen Taylor <otaylor@redhat.com>
- Date
- Sep 14, 2009, 23:18 UTC
- Message-ID
- <1252970294.11581.71.camel@localhost.localdomain>
- In-Reply-To
- <alpine.LNX.2.00.0909141745410.14907@iabervon.org>
On Mon, 2009-09-14 at 18:21 -0400, Daniel Barkalow wrote:
> On Sun, 13 Sep 2009, Owen Taylor wrote:
[...]
Show 6 quoted lines
> I think the classification logic should move to match_refs(), assuming you > mean the ref->nonfastforward and ref->deletion stuff. It would probably > also be worth having a bit for "already up to date". (Note that > cmd_send_pack() calls match_refs(), so there wouldn't have to be > duplication between the legacy cmd_send_pack() code path and the > transport_push() codepath if the code moved into match_refs()).
[...]
Show 12 quoted lines
> > - You add another vfunc to the transport - '->get_capabilities' or
> > something - that encapsulates server_supports("delete-refs").
>
> I think it would be better to have a vfunc that takes refs with the
> classification bits set and sets the statuses based on the idea that we're
> not going to lose any races and the remote won't reject our change for
> some reason we don't know about. There's a potentially large and varied
> set of restrictions on what the other side is willing to accept, and I
> think it would be better to put that on the other side of the vfunc,
> rather than having the main transport code know that "delete-refs" means
> that you can delete refs, "nonfastforward" means you can force a
> non-fast-forward, something means you can create files named "CVS", etc.match_refs seems like a reasonable place to put this logic, but I'm not sure I completely follow what you are proposing in terms of transport-specific customization.
match_refs() is called from a couple of places where there is no 'transport' (cmd_send_pack() and http-push.c) so it can't itself call into the transport code.
Are you thinking of a virtual function that would be a "second pass" after the main logic done is done by match_refs; so ->check_refs() virtual function?
How would the 'bit for "already up to date"' differ from REF_STATUS_UPTODATE. ?
Show 6 quoted lines
> I think a pre-push hook would be popular; I know I'd like to have a hook > that makes sure that I signed off anything I'm pushing (when the server > might check that *somebody* did, but wouldn't know that this push is > supposed to be me), and I'd like a hook that checks that I've referenced > an issue in an issue tracker for each commit that I'm pushing (but only > when I go to push it).
If I can figure out the rest of it, I'll look at adding a hook on top as a sweetener :-)
- Owen