Re: [PATCH 0/9] setup_git_directory(): return to original cwd upon reaching .git
- From
Nguyen Thai Ngoc Duy <pclouds@gmail.com>
- Date
- Jul 24, 2010, 11:45 UTC
- Message-ID
- <AANLkTik44c_piRQopbK9utUS30E37LnjTMLs_Wp_8TV9@mail.gmail.com>
- In-Reply-To
- <20100724111505.GC7150@burratino>
2010/7/24 Jonathan Nieder <jrnieder@gmail.com>:
Show 13 quoted lines
> 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.
Show 7 quoted lines
> 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