On 7/9/2018 3:08 PM, Junio C Hamano wrote:
Derrick Stolee <sto...@gmail.com> writes:
diff --git a/midx.h b/midx.h
index dbdbe9f873..2d83dd9ec1 100644
--- a/midx.h
+++ b/midx.h
@@ -1,6 +1,10 @@
  #ifndef __MIDX_H__
  #define __MIDX_H__
+struct multi_pack_index;
I actually was quite surprised that this struct is defined in
object-store.h and not here.  It feels the other way around.

The raw_object_store needs to know that such an in-core structure
might exist as an optional feature in an object store, but as an
optional feature, I suspect that it has a pointer to an instance of
multi_pack_index, instead of embedding the struct itself in it, so I
would have expected to see an "I am only telling you that there is a
struct with this name, but I am leaving it opaque as you do not have
any business looking inside the struct yourself.  You only need to
be aware of the type's existence and a pointer to it so that you can
call helpers that know what's inside and that should be sufficient
for your needs." decl like this in object-store.h and instead an
actual implementation in here.

I thought it natural to include the struct definition next to the definition for 'struct packed_git', but I like your separation of concerns. Perhaps we could move the packed_git definition to packfile.h as well (separately). Of course, that sounds like an unnecessary churn.

Thanks,

-Stolee

Reply via email to