From: Jan Durovec Date: Tue, 19 Apr 2016 18:04:23 GMT Subject: Re: [PATCH v2] git-p4: add P4 jobs to git commit message Message-ID: In-Reply-To: Would it be acceptable the other way around? I.e. this patch followed by the one that fixes code style (once this gets merged)? Reason being that I don't know how to use submitGit to generate a patch against a state that is not already in git repo (ie. based on another patch). In the following patch I'll * add spaces before () for functions in t/lib-git-p4.sh * remove name local variable in p4_add_job/user in t/lib-git-p4.sh * fix t/t98* leading tabs where <<- is used On Tue, Apr 19, 2016 at 7:47 PM, Junio C Hamano wrote: > Jan Durovec writes: > >> given the fact that the rest of the code just follows existing >> source code style, i.e. >> >> * using %s not %d to add number to string (see git-p4.py:2301) > > This one I do not care too deeply about, as formatting anything that > can be formatted via '%s' could just be more Pythonic style, in > which case "%s" is perfectly fine. It just didn't look kosher to my > C trained eyes, that's all. > >> * no space between function name and parentheses (see all functions >> in t/lib-git-p4.sh) > > I thought I said "Not a new issue, but..." to this one, and it > appears that leaving <<- here-doc unindented, which is stupid as > that shows the person who is writing the here-doc does not know what > the dash s/he is writing means at all, is also not a new issue. > >> * no tab when specifying in-line expected output (see t/t9826...) > >> ...is there anything left to fix in this patch or is it good as is? >> >> I.e. would you prefer me to change the code mentioned above at the cost >> of code style consistency? > > Not really. > > If you really want to know the preference, we prefer a preliminary > clean-up patch to correct existing style issues, followed by a new > feature patch that builds on the cleaned up codebase. >