[jbosstools-dev] ACTION REQUIRED: making sure commits are in master, help INSIDE!!!

Thu Apr 21 04:55:49 EDT 2016

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?
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.
So with the context eliminated, it would actually be useful I think.
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 ;)

-Martin

> On 20. 4. 2016, at 20:16, Rob Stryker <rob.stryker at redhat.com> wrote:
> 
> 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. 
> 
> 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. 
> 
> 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. 
> 
> 
> [rob at rawbdor jbosstools-integration-tests] (jbosstools-4.3.x)$ git show -U0 --minimal dec2f114af3acce10c5c9d3ee6043016e5322142 | git patch-id
> 
> 42be57b63f6d863843d63e954fcda5a8e589fb8c dec2f114af3acce10c5c9d3ee6043016e5322142
> 30045b1ac7faf3b7a1cfbabc4b2d9d665545813b 0000000000000000000000000000000000000000
> d6fabb7f604aa2ca3a066027d55e7c1251355a38 0000000000000000000000000000000000000000
> 94bfa60cb7c6e65908dd45c52e2c232be4449d30 0000000000000000000000000000000000000000
> 
> [rob at rawbdor jbosstools-integration-tests] (jbosstools-4.3.x)$ git show -U0 --minimal 904fea9f59c54e612aac05ae347126c618fe695d | git patch-id
> 
> 42be57b63f6d863843d63e954fcda5a8e589fb8c 904fea9f59c54e612aac05ae347126c618fe695d
> 30045b1ac7faf3b7a1cfbabc4b2d9d665545813b 0000000000000000000000000000000000000000
> d6fabb7f604aa2ca3a066027d55e7c1251355a38 0000000000000000000000000000000000000000
> 94bfa60cb7c6e65908dd45c52e2c232be4449d30 0000000000000000000000000000000000000000
> 
> 
> 
> 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. 
> 
> 
> 
> On 04/20/2016 11:35 AM, Martin Malina wrote:
>> Great idea, Rob! I would also appreciated if there was a JIRA for each component as Nick suggested.
>> There are 30 reports for integration tests. Looking at a few, it was either just a change in a pom or a false positive.
>> So I wonder why that is.
>> Take these two as an example:
>> https://github.com/jbosstools/jbosstools-integration-tests/commit/dec2f114af3acce10c5c9d3ee6043016e5322142 <https://github.com/jbosstools/jbosstools-integration-tests/commit/dec2f114af3acce10c5c9d3ee6043016e5322142>
>> https://github.com/jbosstools/jbosstools-integration-tests/commit/904fea9f59c54e612aac05ae347126c618fe695d <https://github.com/jbosstools/jbosstools-integration-tests/commit/904fea9f59c54e612aac05ae347126c618fe695d>
>> 
>> Their patch-id is different:
>> $ git show dec2f114|git patch-id
>> 9b70b5088668467b18419301adacb703d26411cc dec2f114af3acce10c5c9d3ee6043016e5322142
>> $ git show 904fea9f5|git patch-id
>> 1748ab1b8d0835a3a6b3d45fc0c57bb4fa24e2a3 904fea9f59c54e612aac05ae347126c618fe695d
>> 
>> Now let's see what seems to be different:
>> $ diff <(git show 904fea9f5) <(git show dec2f114)
>> 1c1
>> < commit 904fea9f59c54e612aac05ae347126c618fe695d
>> ---
>> > commit dec2f114af3acce10c5c9d3ee6043016e5322142
>> 8c8
>> < index 0942ef1..ff618cd 100644
>> ---
>> > index face7e9..40b4e4e 100644
>> 17c17
>> <  Bundle-Version: 4.4.0.qualifier
>> ---
>> >  Bundle-Version: 4.3.0.qualifier
>> 143c143
>> < index ff53ec5..54a64ee 100644
>> ---
>> > index fbccb09..aae2c25 100644
>> 
>> 
>> 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?
>> 
>> -Martin
>> 
>>> On 18. 4. 2016, at 23:09, Rob Stryker < <mailto:rob.stryker at redhat.com>rob.stryker at redhat.com <mailto:rob.stryker at redhat.com>> wrote:
>>> 
>>> On 04/18/2016 05:05 AM, Max Rydahl Andersen wrote:
>>>> 
>>>> great initiative - may I suggest you do a PR for your script and put it at 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> where we have other such utilities. 
>>>> 
>>>> Also, might be easier if you put the run info into  jira since the output is quite hard to read/use here from mail. 
>>>> 
>>>> 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 ? 
>>>> 
>>> 
>>> 
>>> First, I disagree that these are false-positives ;)   These are commits that are in maintenance that aren't in master.  
>>> 
>>> 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. 
>>> 
>>> 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. 
>>> 
>>> Il meglio è nemico del bene  -  Perfect is the enemy of good
>>> 
>>> https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good <https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good>
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> jbosstools-dev mailing list
>>> jbosstools-dev at lists.jboss.org <mailto:jbosstools-dev at lists.jboss.org>
>>> https://lists.jboss.org/mailman/listinfo/jbosstools-dev <https://lists.jboss.org/mailman/listinfo/jbosstools-dev>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/jbosstools-dev/attachments/20160421/cbc21c98/attachment.html 