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

[Rob Stryker]

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/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 <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 
>>> 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
>> /
>> /
>>
>> _______________________________________________
>> 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
>

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