Re: [PATCH 0/9] setup_git_directory(): return to original cwd upon reaching .git

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2010/7/24 Jonathan Nieder <jrnieder@xxxxxxxxx>:
> Hi Duy,
>
> Nguyễn Thái Ngọc Duy wrote:
>
>> Prefix and cwd should be consistent.
>
> I agree with this, but it took me a while to figure out why your fix
> is safe.  Here’s how I figured it out.
>
> Patch 1 cleans up the test script you added a test to.  After cleaning
> it up, it is clearer the test does not belong there.
>
> Patch 2 creates a proper home for the new test.

The test was originally to demostrate the breakage and apply was
chosen because it's the first command I found applicable. I doubt if
only "apply" (and index-pack) were affected.

But anyway, more tests can't harm git.

>
> Patches 3-7 split up setup_git_directory_gently() into small enough
> pieces that a person with a short attention span can read it now.
> No functional change intended.

Yeah. I tempted to clean up setup_*_gently too, by splitting the
discovery path ($GIT_DIR check, chdir up...) and the real setup (bunch
of global variables, prefix, check_format, set_git_dir) apart. The was
painful and did not improve readability as much. I gave up.

> Patch 8 is your fix.  I removed the comment (which was just confusing
> me) and clarified the commit message to compensate

If that makes it easier to understand, I'm OK.

> Patch 9 is your patch to revert the other, now redundant fix, also
> with commit message tweaks.
>
> After this exercise, your patches still look good. :)  Maybe these
> by-products could be useful somehow.
>
> Thoughts (especially improvements) welcome.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]