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/