Stefan Beller wrote: > Signed-off-by: Ronnie Sahlberg <sahlb...@google.com> > Signed-off-by: Stefan Beller <sbel...@google.com> > Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> > ---
Yep, looks good now. Thanks for bearing with me. [...] > +++ b/refs.h > @@ -163,8 +163,16 @@ extern void rollback_packed_refs(void); [...] > +/* > + * Remove the refs listed in the unsorted string list 'without' from > + * the packed-refs file. On error, packed-refs will be unchanged, the > + * return value is nonzero, and a message about the error is written > + * to the 'err' strbuf. > + * > + * The refs in 'without' may be unsorted. > + * 'err' must not be NULL. I think we've gone back and forth enough on this text and it's not worth the transactional cost to tweak it further, so I'm not suggesting a change --- just explaining how I read it for future reference. "may be unsorted" is confusing to me. It sounds like the reader of this comment (someone calling repack_without_refs) has to be prepared for that possibility. But we are saying the opposite --- not "be prepared", but "don't worry about sorting 'without', since repack_without_refs can handle it". It's also redundant, since the paragraph above already says that 'without' is an unsorted string list. The way I see it, there are four types that for various reasons (lack of language-level support for subclassing, etc) are conflated into a single struct in the string-list API: * sorted string list that owns its items (i.e., created with DUP) * sorted string list that does not own its items (i.e., created with NODUP) * unsorted string list that owns its items * unsorted string list that does not own its items Different functions are valid to call on each type, as documented in the comments in string-list.h. repack_without_refs accepts all 4 types of string-list. That's what it means when the documentation says its argument is unsorted. Thanks, Jonathan -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
