Thread (57 messages) 57 messages, 7 authors, 2021-10-12

Re: [PATCH v3 2/7] refs: teach arbitrary repo support to iterators

flat view

From: Jonathan Tan <hidden>
Date: 2021-10-08 20:12:40

quoted
@@ -1139,7 +1142,7 @@ static int should_pack_ref(const char *refname,
 		return 0;
 
 	/* Do not pack broken refs: */
-	if (!ref_resolves_to_object(refname, oid, ref_flags))
+	if (!ref_resolves_to_object(refname, the_repository, oid, ref_flags))
 		return 0;
Nit: It would be nice to have a NEEDSWORK the explain why the_repository is
hardcoded instead of the commit message. Without having something in the
code, it is a little surprising to see the_repository when we are so
close to removing the_repository from refs/ altogether.

This in the spirit of your check in patch 3, where we explicitly warn
readers when non-the_repository is not supported.

  +	if (iter->repo != the_repository)
  +		BUG("peeling for non-the_repository is not supported");
This doesn't bring us closer or further from removing the_repository
from refs/ - originally, this call to ref_resolves_to_object()
implicitly referenced the_repository, and now it explicitly does, so
there's no increase or decrease in usage of the_repository. I think that
the true NEEDSWORK is that the entire ref writing system operates only
on the_repository, and this is probably not the place to note it.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help