On 31/08/09 08:25, Stonewall Ballard wrote:
> It seems that you all aren't using XCode 3.2. Pieter said that it
> changes the xibs, hence produces excessively-large patches. I think
> I'll have to wait until the origin is updated. Might that happen
> anytime soon?
Yes, I'm hoping Pieter will push to GitHub soon too ;) About XCode 3.2: 
I just noticed it is out (though it seems to be targeted at 10.6). I 
ordered Snow Leopard yesterday and will start using it next week when I 
get back home.

Diffs of XIBs are often unnecessarily big, regardless of the 
XCode-version. With some practice one can spot the lines that can be 
left out of a commit.

> I submitted this patch by pasting into a reply panel directly in
> Google Groups. I hoped that would work, but apparently not. Is there a
> better way to avoid hard-wrapping?
That's not a good way to send your patches. You have a few other options

1. Use a mailprogram (Thunderbird, Mail.app, mutt), but make sure that 
copy-pasting patches doesn't rewrap them. Have a look here for hints on 
common mail-programs and their ill-behavin':
http://repo.or.cz/w/git.git?a=blob_plain;f=Documentation/SubmittingPatches;hb=HEAD
2. Use git send-email like this:
git send-email --suppress-cc=self 000* [email protected]
You'll have to know your way around setting up a small MTA (think msmtp 
or sendmail) on your local system.
3. Send patches as attachment (yuk!). This sucks, since we can't review 
them inline.

> Meanwhile, I've put this patch here:
> <http://dl.getdropbox.com/u/297034/0001-Add-preferences-for-opening-
> things-at-launch.patch.zip>
Now, about the patch: It looks fine, one thing one might argue about is 
the wording of the prefs and the placement, but that's something we can 
worry about when we get more preference-options. I can't say much about 
the XIB, but it probably could be a smaller diff. But it applied fine on 
my bugfixes-branch, so no problem here.

One tiny technical detail I was wondering about was:
id curPath = [[[NSProcessInfo processInfo] environment] objectForKey:@"PWD"];
This works fine, but shouldn't we use NSString* curPath = ... here? 
Using a specific type makes it easier for a reviewer to see what kind of 
object is to be expected, and in this case it's clearly a string, last 
but not least because [NSURL fileURLWithPath:curPath] expects an 
NSString* as parameter.

Other than that I have no objections to this patch from the 
usability-standpoint, I even might come to use it myself ;)

Greetings,
Jojo

-- 
Johannes Gilger <[email protected]>
http://heipei.net
GPG-Key: 0x42F6DE81
GPG-Fingerprint: BB49 F967 775E BB52 3A81  882C 58EE B178 42F6 DE81

Reply via email to