Re: [PATCH v2 1/2] check-ref-format --print: Normalize refnames that start with slashes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Aug 27, 2011 at 11:30:26AM -0700, Junio C Hamano wrote:

> It sometimes gets frustrating to see a re-rolled submission that ignores
> the fix-ups to messages and patches I make locally before queued to 'pu'.
> 
> It is easy for me to say that they should fetch 'pu' to see what is queued
> before resubmitting, but I've been wondering if there is a better way to
> communicate back such differences, so that submitters can easily sanity
> check to see if my fix-ups are sensible, and to ensure that the re-rolled
> patches do not discard them by mistake before submitting.

FWIW, I never do this, nor did I realize I was expected to. It takes
effort on the part of the submitter, which means they're probably not
inclined to do so unless there are changes that they know are pending.
I.e., if I see changes as replies on the list, I grab them and put them
into my re-roll. But I am not in the habit of poking through pu, trying
to match up equivalent patches, and then comparing them against my
versions, just on the off chance that some change has been made that
wasn't mentioned on the list. I always assumed that you did one of:

  1. Comment on the patch via email, just as any other reviewer, so it
     can go into the re-roll.

  2. Fix-up the patch or commit message during "am", with the assumption
     that it is ready to be merged at least to "next", at which point
     re-rolls are no longer OK, anyway.

I mentioned "it takes effort" above. I don't mean "submitters shouldn't
be expected to put in extra effort". But we should make sure the effort
is well-spent, which means:

  1. Giving them some indication that you tweaked things during
     application. It doesn't have to be an inclusive list. Even saying
     "Thanks, applied with some spelling fixes" instead of your usual
     "Thanks" is enough (actually, I think you frequently do so
     already).

  2. Having better tool support for picking out the topics. Right now
     we don't know the name of the topic branch you choose without
     hunting for it in pu (or seeing it later in a What's Cooking
     message). And even if we do, picking the tip commit out of pu
     requires a bit of scripting. Have you considered publishing the
     tips of topic branches you apply? Probably it makes sense to keep
     them out of refs/heads/ in git.git, but even having them available
     in refs/topics/ would allow interested parties to fetch them.

  3. Having better tool support for comparing two sets of commits. The
     ideal interface (to me) for this workflow would be something like:

         $ git compare-series my-topic origin/my-topic origin
         Patch 1: first patch subject...OK
         Patch 2: second patch subject...

         diff --git a/hello.c b/hello.c
         index cef8b34..4f08083 100644
         --- a/hello.c
         +++ b/hello.c
         @@ -1,6 +1,6 @@
          #include <stdio.h>
          int main(void)
          {
         -  printf("hello wrold\n!");
         +  printf("hello world\n!");
            return 0;
          }
         Accept change from upstream [y,n,q]?

         Patch 3: third patch subject...

         diff --git a/COMMIT_MSG b/COMMIT_MSG
         index 54c8fa2..fd7b9be 100644
         --- a/COMMIT_MSG
         +++ b/COMMIT_MSG
         @@ -1,3 +1,3 @@
          third patch subject

         -This patch has a commit message with a tpyo in it.
         +This patch has a commit message with a typo in it.

         Accept commit message update from upstream [y,n,q]?

     where the implementation would be something like:

       a. Get two series of commits as $3..$1 and $3..$2.

       b. Try to match commits from series one to series two, ending up
          with some ordered list of pairs like the one below (entries on
          the left would be commit sha1s from series 1; entries on the
          right would be commit sha1s from series 2).

            (P1, P1)  (an unmodified version of a patch)
            (P2, P2') (a modified version of a patch)
            (P3, )    (dropped P3 in series 2)
            (, P4)    (added P4 in series 2)

          The matching would probably involve some text similarity
          analysis of the commit messages (or possibly the patch
          itself, though that can get confused if an early patch is
          tweaked with a change that cascades through the series).

       c. Do a sort of interactive rebase over this list. For unmodified
          pairs, take the commit from either. For modified pairs,
          checkout the commit from series 1, then "checkout -p" the
          commit from series 2 on top of it. The resulting commit is
          then applied to the intermediate rebase result. For modified
          commit messages, do a "git add -p" in a one-off sub-repository
          with only COMMIT_MSG in it, and then use the result as the
          final commit message. For dropped patches, show the patch from
          series 1 and say "Drop this patch?" For added patches, do the
          same (but with "Add this patch?").

          I'm not sure how reordering of patches would be handled, if at
          all. Maybe just as a deletion and an addition.

Anyway, that's just an idea I had while writing this message. I wouldn't
be surprised if there are a ton of awful corner cases I didn't think
about, or that somebody has a much better way of accomplishing the same
thing.

I often end up with something close to this by rebasing my topic
branches on top of master. Once your version hits master, then I see
your changes as conflicts. It's a bit annoying, though, because the
conflicts are frequently annoying to resolve. E.g., you fix a typo in
the line, and then every patch in the series after that which modified
the same line (or even a nearby one) ends up conflicting.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]