Re: [PATCH v5 4/4] builtin/branch: strip refs/heads/ using skip_prefix

Junio C Hamano [off-list ref] writes:

SZEDER Gábor [off-list ref] writes:

quoted

On Fri, Dec 1, 2017 at 6:59 AM, Kaartic Sivaraam
[off-list ref] wrote:

quoted

Sorry, missed a ';' in v4.

The surprising thing I discovered in the TravisCI build for v4
was that apart from the 'Documentation' build the 'Static Analysis'
build passed, with the following output,

-- <snip>
$ ci/run-static-analysis.sh
GIT_VERSION = 2.13.1.1972.g6ced3f745
     SPATCH contrib/coccinelle/array.cocci
     SPATCH result: contrib/coccinelle/array.cocci.patch
     SPATCH contrib/coccinelle/free.cocci
     SPATCH contrib/coccinelle/object_id.cocci
     SPATCH contrib/coccinelle/qsort.cocci
     SPATCH contrib/coccinelle/strbuf.cocci
     SPATCH result: contrib/coccinelle/strbuf.cocci.patch
     SPATCH contrib/coccinelle/swap.cocci
     SPATCH contrib/coccinelle/xstrdup_or_null.cocci

The command "ci/run-static-analysis.sh" exited with 0.

Perhaps Coccinelle should have errored out, or perhaps its 0 exit code
means "I didn't find any code matching any of the semantic patches that
required transformation".

quoted

I guess static analysis tools make an assumption that the source
code is syntactically valid for them to work correctly. So, I guess
we should at least make sure the code 'compiles' before running
the static analysis tool even though we don't build it completely.
I'm not sure if it's a bad thing to run the static analysis on code
that isn't syntactically valid, though.

Travis CI already runs 6 build jobs compiling Git.  And that is in
addition to the one that you should have run yourself before even
thinking about submitting v4 ;)  That's plenty to catch errors like
these.  And if any of those builds fail because Git can't be built or
because of a test failure, then Coccinelle's success doesn't matter at
all, because the commit is toast anyway.

Somehow this fell underneath my radar horizon.  I see v4 and v5 of
4/4 but do not seem to find 1-3/4.  Is this meant to be a standalone
patch, or am I expected to already have 1-3 that we already are
committed to take?

Ah, I am guessing that this would apply on top of 1-3/4 in the
thread with [off-list ref]

The base of the series seems to predate 16169285 ("Merge branch
'jc/branch-name-sanity'", 2017-11-28), so let me see how it looks by
applying those three plus this one on top of 'master' before that
point.

On Friday 08 December 2017 04:44 AM, Junio C Hamano wrote:

Junio C Hamano [off-list ref] writes:

quoted

Somehow this fell underneath my radar horizon.  I see v4 and v5 of
4/4 but do not seem to find 1-3/4.  Is this meant to be a standalone
patch, or am I expected to already have 1-3 that we already are
committed to take?

Ah, I am guessing that this would apply on top of 1-3/4 in the
thread with [off-list ref]

You guessed right; at the right time. I was about to ask why this got 
"out of your radar" in reply to your recent "What's cooking" email :-)

The base of the series seems to predate 16169285 ("Merge branch
'jc/branch-name-sanity'", 2017-11-28), so let me see how it looks by
applying those three plus this one on top of 'master' before that
point.

Let me know if this has terrible conflicts so that I can rebase the 
series on top of 'master'.


Thanks,
Kaartic