On Tue, Sep 19, 2017 at 08:22:08AM +0200, Michael Haggerty wrote: > This branch is also available from my fork on GitHub as branch > `mmap-packed-refs`. > > My main uncertainties are: > > 1. Does this code actually work on Windows? > > 2. Did I implement the new compile-time option correctly? (I just > cargo-culted some existing options.) Is there some existing option > that I could piggy-back off of instead of adding a new one? > > 3. Is a compile-time option sufficient, or would the `mmap()` option > need to be configurable at runtime, or even tested at repository > create time as is done for some other filesystem properties in > `create_default_files()`?
I can't really answer (1) due to lack of knowledge. For (2), I think it's mostly good, though I raised a small nit. For (3), I think this is really an OS restriction and not a filesystem one, so probably build-time is OK (though of course Windows people may be able to correct me). I left a few comments, but overall it looks quite good to me. -Peff
