Thread (100 messages) flat view 100 messages, 7 authors, 2018-07-23

Re: [PATCH v4 0/8] ref-in-want

From: Jonathan Tan <hidden>
Date: 2018-06-25 23:03:18

Changes in v4 are fairly minor.  There are a few documentation changes,
commit message updates, as well as a few small style tweaks based on
reviewer comments.
Patches 4 and 7, which I have commented on previously, look good.

As for patch 2, it still has a missing period in the documentation that
I remarked upon in [1], but I'm not too worried about that. Having said
that, Jonathan Nieder suggested [2]:
Stefan mentioned that the spec says

        * The server MUST NOT send any refs which were not requested
          using 'want-ref' lines.

Can client enforce that?  If not, can the spec say SHOULD NOT for the
server and add a MUST describing appropriate client behavior?
I noticed that you did use "SHOULD NOT" instead of "MUST NOT" - in this
case, you should probably also follow the second part about appropriate
client behavior - it's probably best to document and implement that we
ignore all unwanted refs. But considering this situation, though, I
think it's better to just put "MUST NOT" and have the client enforce
this.

One more thing - I think that the fetch part needs to be tested more. In
particular, test cases similar to that of the upload-pack tests
(multiple ref names, ref name + exact SHA-1), and in addition, handling
of wildcards (for example, a wildcard that expands to nothing and a
wildcard that expands to 2 refs).

[1] https://public-inbox.org/git/20180625174056.53053-1-jonathantanmy@google.com/

[2] https://public-inbox.org/git/20180622230119.GL12013@aiede.svl.corp.google.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help