From aab5ec9e6abe69607f21bdc5ccde835ad7b8f4eb Mon Sep 17 00:00:00 2001 From: Sitaram Chamarty Date: Fri, 4 Mar 2011 16:41:32 +0530 Subject: [PATCH] 'hub' ADC takes patterns for 'list-request', has new 'accept' command (plus a few minor fixes) --- contrib/adc/hub | 42 ++++++++++++++++++++++++++++++++---------- contrib/adc/hub.mkd | 44 ++++++++++++++++++++++++++++++++------------ t/t67-hub | 28 +++++++++++++++++++++++----- 3 files changed, 87 insertions(+), 27 deletions(-) diff --git a/contrib/adc/hub b/contrib/adc/hub index a6e9636..df8d091 100755 --- a/contrib/adc/hub +++ b/contrib/adc/hub @@ -30,6 +30,7 @@ Parent repo owner's commands (repo parent): view-diff parent request-number reject parent request-number fetch parent request-number + accept parent request-number EOF exit 1; @@ -56,6 +57,7 @@ my %dispatch = ( 'view-diff' => \&vd, reject => \&reject, fetch => \&fetch, + accept => \&accept, ); my $cmd = shift || ''; @@ -129,8 +131,8 @@ sub rs { # -------------------- alice's commands sub lr { - # list-requests parent - usage() unless @_ == 1; + # list-requests parent [optional search strings] + usage() unless @_ >= 1; # Alice must have write access to parent, otherwise she can't really # accept a pull request eventually right? @@ -139,7 +141,10 @@ sub lr { my %hub = get_hub(); return unless %hub; - list_hub('', %hub); + # create the search pattern. ADC arg checking is very strict; it doesn't + # allow &, | etc., so we just generate an OR condition out of the pieces + my $patt = join("|", @_); + list_hub($patt, %hub); } sub vr { @@ -224,11 +229,11 @@ sub reject { my ($repo, $n) = @_; my ($child, $ref, %hub) = get_request_N($repo, $n); + map { die "request status is already '$_'\n" if $_ ne 'pending' } $hub{$child}{$ref}{STATUS}; + # the 'cover letter' message comes from STDIN my $cover = join("", <>); - - map { die "request status is already $_\n" if $_ ne 'pending' } $hub{$child}{$ref}{STATUS}; - $hub{$child}{$ref}{STATUS} = 'rejected'; + $hub{$child}{$ref}{STATUS} = "rejected by $ENV{GL_USER}"; $hub{$child}{$ref}{COVER} .= "\n\nRejected. Message to requestor:\n$cover"; dump_hub(%hub); } @@ -239,7 +244,8 @@ sub fetch { my ($repo, $n) = @_; my ($child, $ref, %hub) = get_request_N($repo, $n); - map { die "request status is already $_\n" if $_ ne 'pending' } $hub{$child}{$ref}{STATUS}; + map { die "request status is already '$_'\n" if $_ ne 'pending' } $hub{$child}{$ref}{STATUS}; + print "user $hub{$child}{$ref}{BOB} asked you to\n\tgit fetch $BASE_FETCH_URL/$child $ref\n"; print "hit enter to accept the fetch request or Ctrl-C to cancel..."; <>; @@ -249,7 +255,22 @@ sub fetch { system("git", "update-ref", "-d", "refs/heads/$fetched_ref"); system("git", "fetch", "$ENV{GL_REPO_BASE_ABS}/$child.git", "$ref:$fetched_ref"); - $hub{$child}{$ref}{STATUS} = 'fetched'; + $hub{$child}{$ref}{STATUS} = "fetched by $ENV{GL_USER}"; + dump_hub(%hub); +} + +sub accept { + # accept parent request-number + usage() unless @_ == 2; + my ($repo, $n) = @_; + my ($child, $ref, %hub) = get_request_N($repo, $n); + + map { die "request status is '$_'; must be 'fetched'\n" if $_ !~ /^fetched by / } $hub{$child}{$ref}{STATUS}; + + # the 'cover letter' message comes from STDIN + my $cover = join("", <>); + $hub{$child}{$ref}{STATUS} = "accepted by $ENV{GL_USER}"; + $hub{$child}{$ref}{COVER} .= "\n\nAccepted. Message to requestor:\n$cover"; dump_hub(%hub); } @@ -323,7 +344,7 @@ sub hub_sort { sub list_hub { my ($status, %hub) = @_; - print "#\tchild-repository-name\t(requestor)\tbranch-or-tag-to-pull\tstatus\n----\n"; + my $header = "#\tchild-repository-name\t(requestor)\tbranch-or-tag-to-pull\tstatus\n----\n"; my @hub = hub_sort(%hub); my $sn = 0; for my $pr (@hub) { @@ -331,7 +352,8 @@ sub list_hub { my $child = $pr->{REPO}; my $ref = $pr->{REF}; my $pr_status = $hub{$child}{$ref}{STATUS}; - next if $status and $pr_status ne $status; + next if $status and $pr_status !~ /$status/; + print $header if $header; $header = ''; print "$sn\t$child\t($hub{$child}{$ref}{BOB})\t$ref\t$pr_status\n"; } } diff --git a/contrib/adc/hub.mkd b/contrib/adc/hub.mkd index a45e8af..bc29e4f 100644 --- a/contrib/adc/hub.mkd +++ b/contrib/adc/hub.mkd @@ -70,7 +70,7 @@ The following commands do not cause a fetch, and should be quite fast: in via STDIN [this message is meant to be transient and is not stored long term; use email for more "permanent" communications]. - echo "hi Alice, please pull" | request-pull child b1 [parent] + echo "hi Alice, please pull" | ssh git@server hub request-pull child b1 [parent] If `child` was created by a recent version of the 'fork' ADC (or the KDE 'clone' ADC), which records the name of the parent repo on a fork, and it @@ -81,7 +81,7 @@ The following commands do not cause a fetch, and should be quite fast: made from his repo child to repo parent. (Note we don't say "accepted" but "fetched"; see later for why): - request-status child [parent] + ssh git@server hub request-status child [parent] The second argument is optional the same way as the 3rd argument in the previous command. @@ -95,13 +95,21 @@ The following commands do not cause a fetch, and should be quite fast: originating repo name (child, in our example), the requestor (Bob, here), and the branch/tag-name (b1) being pulled: - list-requests parent + ssh git@server hub list-requests parent + + This command also takes an optional list of search strings that are OR-d + together and matched against the 'status' field. So saying + + ssh git@server hub list-requests parent fetched pending + + would list only items that were 'fetched' or 'pending' (meaning 'accepted' + and 'rejected' would not show up). * Alice views request # 1 waiting to be pulled into parent. Shows the same details as above for that request, followed by the message that Bob typed in when he ran `request-pull`: - view-request parent 1 + ssh git@server hub view-request parent 1 * Alice views the log of the branch she is being asked to pull. Note that this does NOT involve a fetch, so it will be pretty fast. The log starts @@ -110,7 +118,7 @@ The following commands do not cause a fetch, and should be quite fast: `--decorate`, `--boundary`, etc., could be quite useful. However, she can't use any GUI; it has to be 'git log': - view-log parent 1 + ssh git@server hub view-log parent 1 Notice that the repo name Alice supplies is still her own, although the log comes from the child repo that Bob wants Alice to pull from. It's @@ -119,7 +127,7 @@ The following commands do not cause a fetch, and should be quite fast: * Alice views the diff between arbitrary commits on child: - view-diff parent 1 + ssh git@server hub view-diff parent 1 Again, she mentions *her* reponame but the diff's come from `child`. Also note that, due to restrictions on what characters are allowed in arguments @@ -133,11 +141,7 @@ The following commands do not cause a fetch, and should be quite fast: * Alice doesn't like what she sees and decides to reject it. This command expects some text on STDIN as the rejection message: - echo "hi Bob, your patch needs work; see email" | reject parent 1 - -The following commands will actually fetch from child into parent, and may take -time if the changes are large. However all this is on the server so it does -not involve network traffic: + echo "hi Bob, your patch needs work; see email" | ssh git@server hub reject parent 1 * Alice likes what she sees so far and wants to fetch the branch Bob is asking her to pull. Note that we are intentionally not using the word @@ -151,7 +155,23 @@ not involve network traffic: (presumably emailed) comments from Alice. In a way, this is a "remote tracking branch", just like `refs/remotes/origin/b1`. - fetch parent 1 + ssh git@server hub fetch parent 1 + + This command will actually fetch from child into parent, and may take time + when the changes are large. However all this is on the server so it does + not involve network traffic: + + * Alice has fetched the stuff she wants, looked at it/tested it, and decides + to merge it into `parent`. Once that is done, she runs: + + echo "thanks for the frobnitz patch Bob" | ssh git@server hub accept parent 1 + + to let Bob know next time he checks 'request-status'. Like the `reject` + sub-command, this is also just a status update; no actual 'git' changes + happen. + +Notice the sequence of Alice's action commands: it's either 'reject', or a +'fetch' then 'accept'. diff --git a/t/t67-hub b/t/t67-hub index 75d6ffd..3218407 100644 --- a/t/t67-hub +++ b/t/t67-hub @@ -131,7 +131,7 @@ do notexpect "new file mode" expect "invalid SHA:" - name "alice rejects 2, accepts 3" + name "alice rejects 2, fetches 3" echo captain was sober today | runlocal ssh u1 hub reject r1 2 notexpect . echo | runlocal ssh u1 hub fetch r1 3 @@ -143,14 +143,32 @@ do name "bob checks his pending requests" runlocal ssh u2 hub request-status child/u2/myr1 expect "1 child/u2/myr1 (u2) b1 pending" - expect "2 child/u2/myr1 (u2) b2 rejected" - expect "3 child/u2/myr1 (u2) b3 fetched" + expect "2 child/u2/myr1 (u2) b2 rejected by u1" + expect "3 child/u2/myr1 (u2) b3 fetched by u1" name "alice checks her pull requests" runlocal ssh u1 hub list-requests r1 expect "1 child/u2/myr1 (u2) b1 pending" - expect "2 child/u2/myr1 (u2) b2 rejected" - expect "3 child/u2/myr1 (u2) b3 fetched" + expect "2 child/u2/myr1 (u2) b2 rejected by u1" + expect "3 child/u2/myr1 (u2) b3 fetched by u1" + + name "alice checks her pull requests by pattern" + runlocal ssh u1 hub list-requests r1 rej + notexpect "1 child/u2/myr1 (u2) b1 pending" + expect "2 child/u2/myr1 (u2) b2 rejected by u1" + notexpect "3 child/u2/myr1 (u2) b3 fetched by u1" + runlocal ssh u1 hub list-requests r1 pend rej + expect "1 child/u2/myr1 (u2) b1 pending" + expect "2 child/u2/myr1 (u2) b2 rejected by u1" + notexpect "3 child/u2/myr1 (u2) b3 fetched by u1" + + name "alice accepts 3, then checks her pull requests" + echo the rain in spain | runlocal ssh u1 hub accept r1 3 + notexpect . + runlocal ssh u1 hub list-requests r1 + expect "1 child/u2/myr1 (u2) b1 pending" + expect "2 child/u2/myr1 (u2) b2 rejected by u1" + expect "3 child/u2/myr1 (u2) b3 accepted by u1" name "INTERNAL" done