onepremise / gReview

A Bamboo plugin integrating Gerrit which allows Bamboo to verify changes and update the Gerrit review system.
Apache License 2.0
15 stars 23 forks source link

Changes discovering problem #23

Open two2011 opened 11 years ago

two2011 commented 11 years ago

Suppose following git's history:         - s5       / s1 - s2 - s3 - s4       \          - s6

s1 - is the latest commit that has been pushed to review and has been build by Bamboo.

Now I'm pushing commits to review in following order: 1) s2, s3, s4 2) s5 3) s6

Bamboo execute build plan. S6 is the only revision for which new build will be executed.

I think that Bamboo should execute builds for all changes.

I haven't investigated deeply code, but I thin the problem is in: GerritRepositoryAdapter#collectChangesSinceLastBuild (line) 590.

We should not look for the last unverified change but for the first (in the case above it will be s2 -> s3 -> s4 -> s5 -> s6).

P.S. I have question. When I was debugging plugin I've noticed that after (triggered) build ends GerritRepositoryAdapter#collectChangesSinceLastBuild method is invoked again. Is it a part of Bamboo logic or gReview plug-in ?

two2011 commented 11 years ago

Here is a patch: http://kamilsobon.pl/gReview.patch

onepremise commented 11 years ago

Hi Kamil,

I see you've taken interest in our project :) Thanks for reporting the issue and taking the time to help resolve the problem. Are you familiar with pull requests? All you have to do to submit your patch, is fork my project, apply your patch, and then make a pull request. This streamlines the process of project contributions and allows me to review the incoming change.

Once you get your change submitted, I'll download, review, and test your change in our build environment. If all goes well, it should go straight in. Thanks again for taking the time to research the problem and provide solution!

two2011 commented 11 years ago

Hi,

Sorry for no response. I somehow miss e-mail notification about new post in this thread. Nevertheless, I'm not using github in day-to-day development, so I'm not familiar with pull requests. I will try to follow your guides.

P.S. Have you investigated patch that I have provided ?

onepremise commented 11 years ago

Hi Kamil,

I haven't tested it yet. However, i definitely have it in my queue. It's next in line after some updates I'm working on for the Gerrit Team. I'll keep you posted as soon as I test it out. Thanks!

two2011 commented 11 years ago

I've found another "bug". Plugin-in in current state cannot work with multi-stage build. This limitation is due to GerritProcessor that is (if configured) invoked after each stage has completed. Instead, it's logic (setting verification flag to Gerrit's change) should be invoked after build of whole plan is finished.

Currently I'm working on some workaround, but not a pretty one, due to the fact that I'm not familiar with developing Bamboo plug-ins :)