Re: [PATCH 00/23] http*: refactor (v6)

5 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 00/23] http*: refactor (v6)

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:55

Tay Ray Chuan [off-list ref] writes:
Junio, this replaces the entire 'rc/http-push' topic.

Changes from the previous patch series:

- add a list of functions added (patch #15)

- use back the die()'s like in the original (patch #16)
Thanks, will queue, but please double check the result after I push it
out, as I found some other changes not listed above, relative to what was
queued in 'pu' (I am not complaining; I am asking your extra set of
eyeballs in case I somehow screwed up applying your series to my tree).

 - return value of fetch_indices() is 0 when http_get_info_packs() returns
   HTTP_MISSING_TARGET (both in http-push.c and http-walker.c);

 - break the function definition of run_request_queue() by removing "void"
   from the parameter list ("send out fetch requests on queue" -- no need
   to resend, I fixed it up again while applying);

 - update the error checking in transport.c:get_refs_via_curl() to make it
   look more similar to what are in fetch_indices() in both http-push.c and
   http-walker.c.

 - add unused "cleanup:" label to get_refs_via_curl() in transport.c
   (again no need to resend, I fixed it up while applying);

 - add back a few missing "->local = NULL" I screwed up while applying the
   previous series.

By the way, this is not your fault at all, but for a patch series
description, "shortlog" is a sucky way, as we can see below.  When you
talk about "patch #15", the reader cannot find which one you are talking
about but has to go back to the MUA, so the list is useless for that
purpose.

The output of "shortlog" in an e-mail message is wonderful when you are a
subsystem maintainer who bundles many people's work and are sending out a
pull request (i.e.  you are transferring the whole graph in "take it or
reject all" fashion, and you can refer to an individual changeset with its
commit object name).

But I think a cover letter to a patch series should serve as an easy index
from patch number to the title in some way, and "shortlog" is not suitable
for that eapecially for a multi-author series.
Christian Couder (1):
  http-push: fix missing "#ifdef USE_CURL_MULTI" around
    "is_running_queue"

Mike Hommey (7):
  Don't expect verify_pack() callers to set pack_size
  transport.c::get_refs_via_curl(): do not leak refs_url
  http.c: new functions for the http API
  transport.c::get_refs_via_curl(): use the new http API
  http.c::http_fetch_ref(): use the new http API
  http-push.c::remote_exists(): use the new http API
  http-push.c::fetch_symref(): use the new http API

Tay Ray Chuan (15):
  t5540-http-push: test fetching of loose objects
  t5540-http-push: test fetching of packed objects
  http*: cleanup slot->local after fclose
  http-push: send out fetch requests on queue
  t5550-http-fetch: test fetching of packed objects
  http-push, http-walker: style fixes
  http-walker: verify remote packs
  http*: copy string returned by sha1_to_hex
  http-push: do not SEGV after fetching a bad pack idx file
  http*: move common variables and macros to http.[ch]
  http: create function end_url_with_slash
  http*: add http_get_info_packs
  http: use new http API in fetch_index()
  http*: add helper methods for fetching packs
  http*: add helper methods for fetching objects (loose)
 http-push.c           |  579 ++++++++---------------------------------------
 http-walker.c         |  533 +++++++------------------------------------
 http.c                |  601 +++++++++++++++++++++++++++++++++++++++++++++++--
 http.h                |   85 +++++++
 pack-check.c          |    8 +-
 t/t5540-http-push.sh  |   36 +++
 t/t5550-http-fetch.sh |    8 +
 transport.c           |   36 ++--
 8 files changed, 905 insertions(+), 981 deletions(-)

Re: [PATCH 00/23] http*: refactor (v6)

From: Jakub Narebski <hidden>
Date: 2016-06-15 22:46:55

Junio C Hamano [off-list ref] writes:
Tay Ray Chuan [off-list ref] writes:
[...]
By the way, this is not your fault at all, but for a patch series
description, "shortlog" is a sucky way, as we can see below.  When you
talk about "patch #15", the reader cannot find which one you are talking
about but has to go back to the MUA, so the list is useless for that
purpose.

The output of "shortlog" in an e-mail message is wonderful when you are a
subsystem maintainer who bundles many people's work and are sending out a
pull request (i.e.  you are transferring the whole graph in "take it or
reject all" fashion, and you can refer to an individual changeset with its
commit object name).

But I think a cover letter to a patch series should serve as an easy index
from patch number to the title in some way, and "shortlog" is not suitable
for that eapecially for a multi-author series.
Before git-send-email learned --cover option, I have used the
following commands to generate "Table of contents" for a patch series:
 * generate patches to some subdirectory using '-o' option 
   of git-format-patch
 * grep for subject and remove "Subject:" line:
   $ grep -h "Subject:" mdir/* | sed -e 's/Subject://' >> mdir/0000-cover.txt

This way I had list of patches in the series in actual order...

P.S. Tay Ray Chuan, this patch series is again not being replies to
     cover letter...
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH 00/23] http*: refactor (v6)

From: Tay Ray Chuan <hidden>
Date: 2016-06-15 22:46:55

Hi,

On Sun, Jun 7, 2009 at 2:04 AM, Junio C Hamano[off-list ref] wrote:
Thanks, will queue, but please double check the result after I push it
out, as I found some other changes not listed above, relative to what was
queued in 'pu' (I am not complaining; I am asking your extra set of
eyeballs in case I somehow screwed up applying your series to my tree).

 - return value of fetch_indices() is 0 when http_get_info_packs() returns
  HTTP_MISSING_TARGET (both in http-push.c and http-walker.c);
This is correct. I had noted this in the patch; perhaps I should have
included it in the cover letter.
 - break the function definition of run_request_queue() by removing "void"
  from the parameter list ("send out fetch requests on queue" -- no need
  to resend, I fixed it up again while applying);
My bad. I hadn't noticed that you've already been doing this.
 - update the error checking in transport.c:get_refs_via_curl() to make it
  look more similar to what are in fetch_indices() in both http-push.c and
  http-walker.c.
Yes. I noted this in the patch text.
 - add unused "cleanup:" label to get_refs_via_curl() in transport.c
  (again no need to resend, I fixed it up while applying);
Indeed. It was an uncaught consequence of the previous change.
 - add back a few missing "->local = NULL" I screwed up while applying the
  previous series.
I'll have to take a look at those.
By the way, this is not your fault at all, but for a patch series
description, "shortlog" is a sucky way, as we can see below.  When you
talk about "patch #15", the reader cannot find which one you are talking
about but has to go back to the MUA, so the list is useless for that
purpose.
Is there a way to do this with git-format-patch? ("this" being a list
of numbered patches, instead of them being grouped by author)

-- 
Cheers,
Ray Chuan

Re: [PATCH 00/23] http*: refactor (v6)

From: Tay Ray Chuan <hidden>
Date: 2016-06-15 22:46:55

Hi,

On Sun, Jun 7, 2009 at 2:23 AM, Jakub Narebski[off-list ref] wrote:
Before git-send-email learned --cover option, I have used the
following commands to generate "Table of contents" for a patch series:
 * generate patches to some subdirectory using '-o' option
  of git-format-patch
 * grep for subject and remove "Subject:" line:
  $ grep -h "Subject:" mdir/* | sed -e 's/Subject://' >> mdir/0000-cover.txt

This way I had list of patches in the series in actual order...
I see. I output my patches into one big mbox file; I'm sure your
technique can be tailored to that.

But then, isn't there a way to do this with git-format-patch or git-something?
P.S. Tay Ray Chuan, this patch series is again not being replies to
    cover letter...
Sorry about that; I already used --thread (you can see that the
patches all have In-Reply-To headers), but the Message-Id of the cover
letter changed while I sent it out, so I messed up.

-- 
Cheers,
Ray Chuan

Re: [PATCH 00/23] http*: refactor (v6)

From: Tay Ray Chuan <hidden>
Date: 2016-06-15 22:46:56

Hi,

On Sun, Jun 7, 2009 at 8:41 AM, Tay Ray Chuan[off-list ref] wrote:
On Sun, Jun 7, 2009 at 2:04 AM, Junio C Hamano[off-list ref] wrote:
quoted
Thanks, will queue, but please double check the result after I push it
out, as I found some other changes not listed above, relative to what was
queued in 'pu' (I am not complaining; I am asking your extra set of
eyeballs in case I somehow screwed up applying your series to my tree).
Looks ok; sorry for the trouble caused.

-- 
Cheers,
Ray Chuan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help