* Re: [PATCH v2] packfile-uri.txt: fix blobPackfileUri description
@ 2021-05-13 6:44 Teng Long
0 siblings, 0 replies; 5+ messages in thread
From: Teng Long @ 2021-05-13 6:44 UTC (permalink / raw)
To: gitster; +Cc: dyroneteng, git, jonathantanmy
Junio C Hamano writes:
>Yes. Some people seem to omit the final response to reviewer
>suggestions on the previous round and just send a revised patch, but
>it is much nicer to cleanly conclude the review cycle for the
>previous round with a separate response (it could just be "yes,
>you're right---I'll incorporate your suggestions in the next round,
>thanks") before starting a new cycle.
If I send a new patch cycle, the "--thread" argument seems
to be recommended. This may be the reason why it is easier
to understand when submitting a series of patches?
I may try to use "--thread", but it may make this whole
patch more confusing, sorry for that.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Optimize the description of the configuration in packfile-uris doc
@ 2021-05-06 21:39 Junio C Hamano
2021-05-11 6:45 ` [PATCH v2] packfile-uri.txt: fix blobPackfileUri description Teng Long
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2021-05-06 21:39 UTC (permalink / raw)
To: Jonathan Tan; +Cc: dyroneteng, git
Jonathan Tan <jonathantanmy@google.com> writes:
>> There is a description problem in the document about packfile-uris. This
>> patch describes the configuration format of `uploadpack.blobPackfileUri`
>> more accurately.
>
> Thanks for noticing this. The diff itself is correct, of course.
>
> As for the commit message, limit the title to 50 characters or fewer if
> possible. Maybe something like "packfile-uri.txt: fix blobPackfileUri
> description" or something like that.
>
> Also in the commit message, maybe mention that the correct format can be
> seen in t5702.
Thanks for giving a review with clear suggestion for improvements,
Jonathan. And thanks for contributing, Teng.
Will look forward to seeing an updated patch.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] packfile-uri.txt: fix blobPackfileUri description
2021-05-06 21:39 [PATCH] Optimize the description of the configuration in packfile-uris doc Junio C Hamano
@ 2021-05-11 6:45 ` Teng Long
2021-05-11 20:50 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Teng Long @ 2021-05-11 6:45 UTC (permalink / raw)
To: gitster; +Cc: dyroneteng, git, jonathantanmy
Fix the 'uploadpack.blobPackfileUri' description in packfile-uri.txt
and the correct format also can be seen in t5702.
Jonathan Tan <jonathantanmy@google.com> writes:
>As for the commit message, limit the title to 50 characters or fewer if
>possible. Maybe something like "packfile-uri.txt: fix blobPackfileUri
>description" or something like that.
Thanks for mention this, "packfile-uri.txt: fix blobPackfileUri
description" is good and meets the "50 characters" requirement. So the
title is modified.
>Also in the commit message, maybe mention that the correct format can be
>seen in t5702.
Because I am implementing another patch[1] about supporting the commit
object in packfile-uri, I noticed the `configure_exclusion` function in
t5702, which is now mentioned in the commit message.
[1]https://public-inbox.org/git/20210507021140.31372-1-dyroneteng@gmail.com
Signed-off-by: Teng Long <dyroneteng@gmail.com>
---
Documentation/technical/packfile-uri.txt | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/technical/packfile-uri.txt b/Documentation/technical/packfile-uri.txt
index f7eabc6c76..1eb525fe76 100644
--- a/Documentation/technical/packfile-uri.txt
+++ b/Documentation/technical/packfile-uri.txt
@@ -35,13 +35,14 @@ include some sort of non-trivial implementation in the Minimum Viable Product,
at least so that we can test the client.
This is the implementation: a feature, marked experimental, that allows the
-server to be configured by one or more `uploadpack.blobPackfileUri=<sha1>
-<uri>` entries. Whenever the list of objects to be sent is assembled, all such
-blobs are excluded, replaced with URIs. As noted in "Future work" below, the
-server can evolve in the future to support excluding other objects (or other
-implementations of servers could be made that support excluding other objects)
-without needing a protocol change, so clients should not expect that packfiles
-downloaded in this way only contain single blobs.
+server to be configured by one or more `uploadpack.blobPackfileUri=
+<object-hash> <pack-hash> <uri>` entries. Whenever the list of objects to be
+sent is assembled, all such blobs are excluded, replaced with URIs. As noted
+in "Future work" below, the server can evolve in the future to support
+excluding other objects (or other implementations of servers could be made
+that support excluding other objects) without needing a protocol change, so
+clients should not expect that packfiles downloaded in this way only contain
+single blobs.
Client design
-------------
--
2.31.1.443.g1f2fb5c85f.dirty
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] packfile-uri.txt: fix blobPackfileUri description
2021-05-11 6:45 ` [PATCH v2] packfile-uri.txt: fix blobPackfileUri description Teng Long
@ 2021-05-11 20:50 ` Junio C Hamano
2021-05-12 14:10 ` Long Teng
0 siblings, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2021-05-11 20:50 UTC (permalink / raw)
To: Teng Long; +Cc: git, jonathantanmy
Teng Long <dyroneteng@gmail.com> writes:
> Fix the 'uploadpack.blobPackfileUri' description in packfile-uri.txt
> and the correct format also can be seen in t5702.
>
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>>As for the commit message, limit the title to 50 characters or fewer if
>>possible. Maybe something like "packfile-uri.txt: fix blobPackfileUri
>>description" or something like that.
>
> Thanks for mention this, "packfile-uri.txt: fix blobPackfileUri
> description" is good and meets the "50 characters" requirement. So the
> title is modified.
>
>>Also in the commit message, maybe mention that the correct format can be
>>seen in t5702.
>
> Because I am implementing another patch[1] about supporting the commit
> object in packfile-uri, I noticed the `configure_exclusion` function in
> t5702, which is now mentioned in the commit message.
>
> [1]https://public-inbox.org/git/20210507021140.31372-1-dyroneteng@gmail.com
>
> Signed-off-by: Teng Long <dyroneteng@gmail.com>
> ---
It seems that the above needs a bit more polishing?
I am not sure if moving the sign-off higher and inserting a
three-dash line before "Jonathan Tan writes" would be sufficient,
but with everything under that quoted material does not seem to
belong to a proposed commit log message proper.
Thanks.
> Documentation/technical/packfile-uri.txt | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/technical/packfile-uri.txt b/Documentation/technical/packfile-uri.txt
> index f7eabc6c76..1eb525fe76 100644
> --- a/Documentation/technical/packfile-uri.txt
> +++ b/Documentation/technical/packfile-uri.txt
> @@ -35,13 +35,14 @@ include some sort of non-trivial implementation in the Minimum Viable Product,
> at least so that we can test the client.
>
> This is the implementation: a feature, marked experimental, that allows the
> -server to be configured by one or more `uploadpack.blobPackfileUri=<sha1>
> -<uri>` entries. Whenever the list of objects to be sent is assembled, all such
> -blobs are excluded, replaced with URIs. As noted in "Future work" below, the
> -server can evolve in the future to support excluding other objects (or other
> -implementations of servers could be made that support excluding other objects)
> -without needing a protocol change, so clients should not expect that packfiles
> -downloaded in this way only contain single blobs.
> +server to be configured by one or more `uploadpack.blobPackfileUri=
> +<object-hash> <pack-hash> <uri>` entries. Whenever the list of objects to be
> +sent is assembled, all such blobs are excluded, replaced with URIs. As noted
> +in "Future work" below, the server can evolve in the future to support
> +excluding other objects (or other implementations of servers could be made
> +that support excluding other objects) without needing a protocol change, so
> +clients should not expect that packfiles downloaded in this way only contain
> +single blobs.
>
> Client design
> -------------
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] packfile-uri.txt: fix blobPackfileUri description
2021-05-11 20:50 ` Junio C Hamano
@ 2021-05-12 14:10 ` Long Teng
2021-05-12 23:32 ` Junio C Hamano
0 siblings, 1 reply; 5+ messages in thread
From: Long Teng @ 2021-05-12 14:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Tan
>It seems that the above needs a bit more polishing?
>
>I am not sure if moving the sign-off higher and inserting a
>three-dash line before "Jonathan Tan writes" would be sufficient,
>but with everything under that quoted material does not seem to
>belong to a proposed commit log message proper.
Sorry, I misunderstood.
I looked at some patches in the community. If I reply to the
reviewer’s suggestion separately and then submit a new patch, is it
the recommended way? (Distinguish between the ‘reply‘ and the
'patch').
Another question is, if I need to continue to complete this patch,
what do I need to do? I think it is to reply to Jonathan Tan
separately, and then resubmit Patch v2. Is this way correct?
Thanks for your reply.
Junio C Hamano <gitster@pobox.com> 于2021年5月12日周三 上午4:50写道:
>
> Teng Long <dyroneteng@gmail.com> writes:
>
> > Fix the 'uploadpack.blobPackfileUri' description in packfile-uri.txt
> > and the correct format also can be seen in t5702.
> >
> > Jonathan Tan <jonathantanmy@google.com> writes:
> >
> >>As for the commit message, limit the title to 50 characters or fewer if
> >>possible. Maybe something like "packfile-uri.txt: fix blobPackfileUri
> >>description" or something like that.
> >
> > Thanks for mention this, "packfile-uri.txt: fix blobPackfileUri
> > description" is good and meets the "50 characters" requirement. So the
> > title is modified.
> >
> >>Also in the commit message, maybe mention that the correct format can be
> >>seen in t5702.
> >
> > Because I am implementing another patch[1] about supporting the commit
> > object in packfile-uri, I noticed the `configure_exclusion` function in
> > t5702, which is now mentioned in the commit message.
> >
> > [1]https://public-inbox.org/git/20210507021140.31372-1-dyroneteng@gmail.com
> >
> > Signed-off-by: Teng Long <dyroneteng@gmail.com>
> > ---
>
> It seems that the above needs a bit more polishing?
>
> I am not sure if moving the sign-off higher and inserting a
> three-dash line before "Jonathan Tan writes" would be sufficient,
> but with everything under that quoted material does not seem to
> belong to a proposed commit log message proper.
>
> Thanks.
>
> > Documentation/technical/packfile-uri.txt | 15 ++++++++-------
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/Documentation/technical/packfile-uri.txt b/Documentation/technical/packfile-uri.txt
> > index f7eabc6c76..1eb525fe76 100644
> > --- a/Documentation/technical/packfile-uri.txt
> > +++ b/Documentation/technical/packfile-uri.txt
> > @@ -35,13 +35,14 @@ include some sort of non-trivial implementation in the Minimum Viable Product,
> > at least so that we can test the client.
> >
> > This is the implementation: a feature, marked experimental, that allows the
> > -server to be configured by one or more `uploadpack.blobPackfileUri=<sha1>
> > -<uri>` entries. Whenever the list of objects to be sent is assembled, all such
> > -blobs are excluded, replaced with URIs. As noted in "Future work" below, the
> > -server can evolve in the future to support excluding other objects (or other
> > -implementations of servers could be made that support excluding other objects)
> > -without needing a protocol change, so clients should not expect that packfiles
> > -downloaded in this way only contain single blobs.
> > +server to be configured by one or more `uploadpack.blobPackfileUri=
> > +<object-hash> <pack-hash> <uri>` entries. Whenever the list of objects to be
> > +sent is assembled, all such blobs are excluded, replaced with URIs. As noted
> > +in "Future work" below, the server can evolve in the future to support
> > +excluding other objects (or other implementations of servers could be made
> > +that support excluding other objects) without needing a protocol change, so
> > +clients should not expect that packfiles downloaded in this way only contain
> > +single blobs.
> >
> > Client design
> > -------------
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] packfile-uri.txt: fix blobPackfileUri description
2021-05-12 14:10 ` Long Teng
@ 2021-05-12 23:32 ` Junio C Hamano
0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2021-05-12 23:32 UTC (permalink / raw)
To: Long Teng; +Cc: git, Jonathan Tan
Long Teng <dyroneteng@gmail.com> writes:
>>It seems that the above needs a bit more polishing?
>>
>>I am not sure if moving the sign-off higher and inserting a
>>three-dash line before "Jonathan Tan writes" would be sufficient,
>>but with everything under that quoted material does not seem to
>>belong to a proposed commit log message proper.
>
> Sorry, I misunderstood.
>
> I looked at some patches in the community. If I reply to the
> reviewer’s suggestion separately and then submit a new patch, is it
> the recommended way? (Distinguish between the ‘reply‘ and the
> 'patch').
Yes. Some people seem to omit the final response to reviewer
suggestions on the previous round and just send a revised patch, but
it is much nicer to cleanly conclude the review cycle for the
previous round with a separate response (it could just be "yes,
you're right---I'll incorporate your suggestions in the next round,
thanks") before starting a new cycle.
And the "patch" side should be written to be understandable even by
those who do not have access to the review history of the previous
round(s)---imagine how it appears in "git log" output to those who
did not read the discussion on this mailing list, and write for
them.
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-13 6:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-13 6:44 [PATCH v2] packfile-uri.txt: fix blobPackfileUri description Teng Long
-- strict thread matches above, loose matches on Subject: below --
2021-05-06 21:39 [PATCH] Optimize the description of the configuration in packfile-uris doc Junio C Hamano
2021-05-11 6:45 ` [PATCH v2] packfile-uri.txt: fix blobPackfileUri description Teng Long
2021-05-11 20:50 ` Junio C Hamano
2021-05-12 14:10 ` Long Teng
2021-05-12 23:32 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).