<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">OK, good. It's unclear to me why "git show -U0" displays 3 lines. But putting that aside, what's stopping you from adding "|head -n1" to the patch-id command?<div class="">BTW, I did check all the 30 commits that you reported for integration tests and indeed they were all false positives - maybe 1/2 was commits that I could find in master, but the diff context made patch-id report them. The rest was either changes to poms or versions in plugins.</div><div class="">So with the context eliminated, it would actually be useful I think.</div><div class="">Although our integration-tests gatekeepers are pretty strict at always requiring 2 PRs, so the risk of a commit missing in master is pretty low to begin with ;)<br class=""><div class=""><br class="webkit-block-placeholder"></div><div class="">-Martin</div>
<br class=""><div><blockquote type="cite" class=""><div class="">On 20. 4. 2016, at 20:16, Rob Stryker <<a href="mailto:rob.stryker@redhat.com" class="">rob.stryker@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
<div text="#000000" bgcolor="#FFFFFF" class="">
<div class="moz-cite-prefix">After a bit more investigation, I found
that we can change the git show command to include -U1 (which
means show only 1 line of surrounding context). This will limit
the amount of times 'context' matters. <br class="">
<br class="">
So I'm sure the next question is, why can't we simply -U0 and
show no surrounding context, before passing into git patch-id?
Well, it seems when I do that, the results of git patch-id are
several lines... which breaks the current implementation. <br class="">
<br class="">
However, it does come up with identical results for git patch-id
if done that way, at least for the first segments of every line. <br class="">
<br class="">
<br class="">
[rob@rawbdor jbosstools-integration-tests] (jbosstools-4.3.x)$ git
show -U0 --minimal dec2f114af3acce10c5c9d3ee6043016e5322142 | git
patch-id<br class="">
<br class="">
42be57b63f6d863843d63e954fcda5a8e589fb8c
dec2f114af3acce10c5c9d3ee6043016e5322142<br class="">
30045b1ac7faf3b7a1cfbabc4b2d9d665545813b
0000000000000000000000000000000000000000<br class="">
d6fabb7f604aa2ca3a066027d55e7c1251355a38
0000000000000000000000000000000000000000<br class="">
94bfa60cb7c6e65908dd45c52e2c232be4449d30
0000000000000000000000000000000000000000<br class="">
<br class="">
[rob@rawbdor jbosstools-integration-tests] (jbosstools-4.3.x)$ git
show -U0 --minimal 904fea9f59c54e612aac05ae347126c618fe695d | git
patch-id<br class="">
<br class="">
42be57b63f6d863843d63e954fcda5a8e589fb8c
904fea9f59c54e612aac05ae347126c618fe695d<br class="">
30045b1ac7faf3b7a1cfbabc4b2d9d665545813b
0000000000000000000000000000000000000000<br class="">
d6fabb7f604aa2ca3a066027d55e7c1251355a38
0000000000000000000000000000000000000000<br class="">
94bfa60cb7c6e65908dd45c52e2c232be4449d30
0000000000000000000000000000000000000000<br class="">
<br class="">
<br class="">
<br class="">
For now, though the current implementation will be broken by using
-U0 because we do not expect multiple line output. I'll test a bit
more and see if I can improve this for next time. <br class="">
<br class="">
<br class="">
<br class="">
On 04/20/2016 11:35 AM, Martin Malina wrote:<br class="">
</div>
<blockquote cite="mid:2D282B6F-7E76-4DCB-98E5-66F7882D4FAD@redhat.com" type="cite" class="">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252" class="">
Great idea, Rob! I would also appreciated if there was a JIRA for
each component as Nick suggested.
<div class="">There are 30 reports for integration tests. Looking
at a few, it was either just a change in a pom or a false
positive.</div>
<div class="">So I wonder why that is.</div>
<div class="">Take these two as an example:</div>
<div class=""><a moz-do-not-send="true" href="https://github.com/jbosstools/jbosstools-integration-tests/commit/dec2f114af3acce10c5c9d3ee6043016e5322142" class="">https://github.com/jbosstools/jbosstools-integration-tests/commit/dec2f114af3acce10c5c9d3ee6043016e5322142</a></div>
<div class=""><a moz-do-not-send="true" href="https://github.com/jbosstools/jbosstools-integration-tests/commit/904fea9f59c54e612aac05ae347126c618fe695d" class="">https://github.com/jbosstools/jbosstools-integration-tests/commit/904fea9f59c54e612aac05ae347126c618fe695d</a></div>
<div class=""><br class="">
</div>
<div class="">Their patch-id is different:</div>
<div class="">$ git show dec2f114|git patch-id<br class="">
9b70b5088668467b18419301adacb703d26411cc
dec2f114af3acce10c5c9d3ee6043016e5322142</div>
<div class="">$ git show 904fea9f5|git patch-id<br class="">
1748ab1b8d0835a3a6b3d45fc0c57bb4fa24e2a3
904fea9f59c54e612aac05ae347126c618fe695d</div>
<div class=""><br class="">
</div>
<div class="">Now let's see what seems to be different:</div>
<div class="">$ diff <(git show 904fea9f5) <(git show
dec2f114)</div>
<div class="">1c1<br class="">
< commit 904fea9f59c54e612aac05ae347126c618fe695d<br class="">
---<br class="">
> commit dec2f114af3acce10c5c9d3ee6043016e5322142<br class="">
8c8<br class="">
< index 0942ef1..ff618cd 100644<br class="">
---<br class="">
> index face7e9..40b4e4e 100644<br class="">
17c17<br class="">
< Bundle-Version: 4.4.0.qualifier<br class="">
---<br class="">
> Bundle-Version: 4.3.0.qualifier<br class="">
143c143<br class="">
< index ff53ec5..54a64ee 100644<br class="">
---<br class="">
> index fbccb09..aae2c25 100644<br class="">
<br class="">
</div>
<div class=""><br class="">
</div>
<div class="">You see, there is nothing different - apart from
checksums, it's the bundle version. But that is outside of the
changes - that is just there for context, not part of the
commits. Any idea why the patch-id is different?</div>
<div class=""><br class="">
</div>
<div class="">-Martin</div>
<div class="">
<br class="">
<div class="">
<blockquote type="cite" class="">
<div class="">On 18. 4. 2016, at 23:09, Rob Stryker <<a moz-do-not-send="true" href="mailto:rob.stryker@redhat.com" class=""></a><a class="moz-txt-link-abbreviated" href="mailto:rob.stryker@redhat.com">rob.stryker@redhat.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
<div text="#000000" bgcolor="#FFFFFF" class="">
<div class="moz-cite-prefix">On 04/18/2016 05:05 AM, Max
Rydahl Andersen wrote:<br class="">
</div>
<blockquote cite="mid:4A70E7DC-6C51-45CC-B54B-9177FAC346E3@redhat.com" type="cite" class=""><br class="">
great initiative - may I suggest you do a PR for your
script and put it at <a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://github.com/jbosstools/jbosstools-build-ci/tree/jbosstools-4.3.x/util">https://github.com/jbosstools/jbosstools-build-ci/tree/jbosstools-4.3.x/util</a>
where we have other such utilities. <br class="">
<br class="">
Also, might be easier if you put the run info into
jira since the output is quite hard to read/use here
from mail. <br class="">
<br class="">
btw. it looks like this tool actually is very good at
finding the false positives - i.e. change in pom.xml
where it just changes version seem to be something you
could filter out somehow ? <br class="">
<br class="">
</blockquote>
<br class="">
<br class="">
First, I disagree that these are false-positives ;)
These are commits that are in maintenance that aren't in
master. <br class="">
<br class="">
And, in all honesty, I think it'd be a mistake to simply
filter them out. I think it's much better to list the
false positive and let the component owner use his
judgment whether the patch needs further inspection or
not. Such simple version-changes will be very very easy
for a human to spot as irrelevant to master. This may
cause the repository owner to waste 2-3 seconds for each
commit, but it guarantees that every possible unmatched
commit is found. <br class="">
<br class="">
I think this is much better and safer than possibly
adding some logic to filter out version changes and
later discover that the logic was wrong and so it was
hiding legitimately missing commits from being shown.
Also, since we're not inspecting the patches at all, but
rather comparing patch-id's, it would make the script
much much more complicated. <br class="">
<br class="">
<i class="">Il meglio è nemico del bene - Perfect is
the enemy of good<br class="">
<br class="">
</i><a moz-do-not-send="true" class="moz-txt-link-freetext" href="https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good">https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good</a><br class="">
<i class=""><br class="">
</i><br class="">
<br class="">
</div>
_______________________________________________<br class="">
jbosstools-dev mailing list<br class="">
<a moz-do-not-send="true" href="mailto:jbosstools-dev@lists.jboss.org" class="">jbosstools-dev@lists.jboss.org</a><br class="">
<a class="moz-txt-link-freetext" href="https://lists.jboss.org/mailman/listinfo/jbosstools-dev">https://lists.jboss.org/mailman/listinfo/jbosstools-dev</a></div>
</blockquote>
</div>
<br class="">
</div>
</blockquote>
<br class="">
</div>
</div></blockquote></div><br class=""></div></body></html>