Re: [PATCH v5 1/2] setup: distingush ENOENT from other stat errors

[Tian Yuchen <a3205153416@gmail.com>]

Hi Karthik,

Thanks for the review!

> Nit: For this function it should be okay to return early. But I was
> expecting a break here, since it was using 'break' before, ideally we
> shouldn't change it unless there is a reason to.

> Would it make more sense to do 'not a regular file: %s'?

Points taken.

> So why didn't we add the tests here for the changes made?

I think the reason is that this commit is a refactoring of the internel 
error handling. The actual logic change that triggers these new paths 
happens in the next commit (2/2). Without the logic change in the next 
patch, the system behaves identically to before, doesn't it?

> Nit: I would even go further to even separate this into two commits:
> 1. Split 'stat()' error into  ERR_STAT_FAILED and ERR_STAT_ENOENT.
> 2. Introduce 'READ_GITFILE_ERR_IS_A_DIR'.
> 
> But I'll leave that to you.
I appreciate the suggestion. But since the changes are relatively small 
and closely related, I'll stick to keeping them in this single patch to 
avoid excessive fragmentation.

Regards,

Yuchen