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.