On Tue, Feb 26, 2008 at 4:23 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jay Soffian <jaysoffian@xxxxxxxxx> writes: > > > * There is one semantic change. You can't use "-s=<strategy>" anymore. That's not > > really proper usage of a short option (it's either "-s<strategy>" or > > "-s <strategy>"). Is it okay to not accept the "-s=<strategy>" form? > > Well, with my maintainer hat on, I must resist _any_ change ;-). > Personally I would not mind this. It is a borderline between > regression and making the option parameters more consistent. My reasoning was that were the script converted to a built-in, the -s= behavior would not likely be maintained (or even noticed...). And that form certainly isn't documented. I think it was an accident that it ever worked. > If a contributor feels wasting his time, what should reviewers > feel reviewing such patches ;-)? I get the smiley, but let me rephrase: is there a long term goal to replace all shell scripts with builtins? And to answer your rhetorical question, reviewers should feel appreciated, because they are. > While it is technically correct that you _can_ feed any option > meant for git-fetch to this command, some options do not make > any sense in the context of git-pull, and we should not > advertise them, or better yet, actively reject them if you can. Well then we have a documentation issue then, because it was from the git pull docs that I wrote the option spec. So I suppose this patch should include a documentation cleanup at the same time. > Because the loop breaks here, the option description above > should mention that options meant for fetch should come after > all the options you want to give to git-pull itself. For > example, I do not think "git pull -q -s stupid $there $that" is > meant to work. > > A better long-term alternative would be to lift that restriction. Sigh, nothing is ever as simple as it seems. And here I was just trying to improve the usage statement. :-( > I do not recall offhand but does the parse-options reorder the > options in any way? If that is the case, it makes the above not > a long-term thing but a must-be-done in a patch that starts to > use parse-options. I don't think it does. Back to the drawing board... j. - 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
