Re: [PATCH v2 2/5] enter_repo: avoid duplicating logic, use is_git_directory() instead
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Sep 14, 2015, 11:57 UTC
- Message-ID
- <CACsJy8D+3o-SKq-MAC1+KzRzMjcU6GUij1OokLWznfZ8V7zUmA@mail.gmail.com>
- In-Reply-To
- <xmqqlhcbxh7l.fsf@gitster.mtv.corp.google.com>
On Sun, Sep 13, 2015 at 8:04 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 34 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes: > >> On Mon, Sep 7, 2015 at 11:33 PM, Junio C Hamano <gitster@pobox.com> wrote: >>> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: >>> >>>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> >>>> --- >>> >>> The cover letter talks about "local clone", and in this entire >>> series, I saw new tests only for the local case, but doesn't this >>> and the next change also affect the case where a Git daemon or a >>> upload-pack process is serving the remote repository? >>> >>> And if so, how is that case affected? >> >> People who serve .git-dir repos should not be affected (I think we >> have enough test cases covering that). People can serve .git-file >> repos as well, which is sort of tested in the local clone test case >> because upload-pack is involved for providing remote refs, I think. > > Unfortunately, the above is still not unclear to me. > > Was serving from a linked repository working without these five > patches, i.e. was the local case the only one that was broken and > needed fixing with these five patches? If so, the log message > should mention that (i.e. "remote case was working OK but local was > broken because ...; change this and that to make local one work as > well"). If the remote case also was broken and fixed by these five > patches, then that is also worth mentioning the same way. > > I didn't ask you to explain it to me in the first place in a > response. The review comment pointed out that the proposed log > message was unclear and those who will be reading "git log" output > need clearer description.
I know. I sent the re-roll before receiving this. I think I still haven't mentioned the impact on remote case. Another update coming, maybe next weekend.
-- Duy