On Thu, Aug 24, 2017 at 2:14 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: > On Wed, Aug 23, 2017 at 5:36 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote: >> The "submodule" argument in this function is a path, which can have >> either '/' or '\\' as a separator. Use is_dir_sep() to support both. >> >> Noticed-by: Johannes Sixt <j6t@xxxxxxxx> >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> > > Immediate questions that come to mind: > * Could this go in as an independent bug fix? It probably could. But it may depend on other submodule changes in this series. > -> If so do we have any test that fails or stops failing on Windows? It was spotted during the review [1] so I guess no test fails on Windows (not that I would know because I can't run tests on Windows) > -> If not, do we need one? Since I don't use Windows, I don't particularly care. And I can't add one anyway because I can't run it. [1] https://public-inbox.org/git/%3Ca74cf309-fb16-2f45-8189-d1d0c655dea4@xxxxxxxx%3E/ > * Assuming this is not an independent bug fix: > Why do we need this patch in this series here? > (I thought this is about worktrees, not submodules. > So does this fix a potential bug that will be exposed > later in this series, but was harmless up to now?) The series could probably be split in two. The first part is about using the new refs_* API in revision.c. This helps clean up refs API a bit, all *_submodule() functions will be one. Not strictly needed to be part of this, it's just a continuation of my previous series that introduces *_refs. Since submodule code is touched, this is found. The second part is actually fixing the prune bug. Should I split the series in two? I think I separated two parts in the past (maybe I misremember) at least in the description... -- Duy
