Support GitHub pull request notifications in Bamboo to implement gated check-ins

[Thomas Nagy]

Is supporting pull request hooks planned in some version of Bamboo? I.e., to effectively implement gated checkins (automatic merging on success).

[fernando palomo]

sad to read this guys...

A nice tool that doesn't understand the way the world is running is never going to be massively used.

[PiotrA]

Yes and no.

No beacuse we don't plan integrating with GitHub pull requests mechanisms directly (AFAIK).

Yes because Bamboo 4.0 is going to introduce a 'multiple branches' concept which will include automatic merges + push on success features. Care me to expand this a little?

[Thomas Nagy]

Thank you for the information. The mentioned automatic merge feature would already come in handy to many. However, if I understand correctly, strong enforcement of policy is not possible without pull requests between different repositories (forks). Consider the following workflow for simulating gated check-in via Git.

Let there be a main repository with devel and stage branches. The developers would only have pull access to the main repository, while Bamboo would have both push and pull access. To implement a feature, the developer would fork the main code to his own area.

These personal feature areas have separate Bamboo plans for continuous testing. If anything breaks in the development process by the continuous integration standard, the commiter is responsible for fixing it in their own area. When a developer sees a feature or a days work to be complete in a sense that it can be readily integrated with the mainline code and is up to coding standards, the developer files a pull requests from the personal area/branch to the mainline devel.

Bamboo could act on this request event in the following way. It would try the pull to a scratch space and if successful, run all regular tests and only push the changes to devel if this is also successful. Otherwise a notification is to be sent (and the pull request could be rejected).

Also of consideration for stage is the following. The mainline devel gets automatically merged to stage by Bamboo on request. However, occasionally, bugfixes should also be allowed probably via similar pull requests. It would be advantageous to help enforcement for this branch by giving the option to only have a queue of successful merges in Bamboo which would only get pushed back after the programmer lead reviews the modifications and chooses to apply.

Would this be compatible with your aims or could you propose modifications to the above to better fit your system?

Thank you for your time.

[PiotrA]

Hm... I see you're describing two separate concepts: the one described in the paragraphs up to "Bamboo could act on this request(...)", and the second one described in "Also of consideration for stage(...)" paragraph. I'll comment them separately.

The first concept, the one where a developer forks the mainline repository, works on a branch and after finishing it asks Bamboo to perform the pull request - I think it's the core concept we're trying to cover with 4.0. Maybe supporting the scheme you're describing won't be a bugless from the beginning, but I think with 4.0 you will be able to achieve it anyway. However Bamboo won't operate on 'pull requests' but rather on "merge&commit" thingy. But I reckon in the end it is the same thing, isn't it?

It will work that way:

The developer forks the mainline repository, and in Bamboo the developer create a PlanBranch for the mainline Plan - that PlanBranch inherits all the mainline Plan's configuration, however the developer is able to override source repository - he/she would input there his/her forked repository url. This assumes the developer have some admin privileges on Bamboo, at least for creating mainline PlanBranches.

The developer works, pushes to his/her repo, Bamboo tests the code - when the developer is ready to merge his work into mainline he would enable 'branch integration' in his/her PlanBranch configuration, specyfing to merge with mainline and enabling "push on successful build". And the Bamboo should take care of the rest and refuse to push if something goes wrong during the merge. The only thing I'm not sure at this moment is whether the merge&push will work smoothly when doing the work on the same branch - 4.0 is designed assuming the developers are working on personal 'feature branches' not personal 'feature forked-repos'.

----====----====----

to be continued...

[PiotrA]

As for the "Also of consideration for stage is the following. The mainline devel gets automatically merged to stage by Bamboo on request." concept. Um... I don't fully understand how it is supposed to work. I mean I have little experience in those whole DVCS concepts and I got lost after reading "help enforcement for this branch by giving the option to only have a queue of successful merges in Bamboo" - can you elaborate a little bit more? Possibly by giving some examples - I don't have a clear picture how things are supposed to work in this scheme, so I'll refrain from commenting whether it will possible in 4.0 or not at all.

Oh, and by the way: there is 4.0 EAP available on DAC - you can grab it and play with the 4.0 features already if you feel bold enough :)

[Thomas Nagy]

The proposed solution is getting closer. We have considered using feature branches previously, though we've decided against them for now as some of our processes do not fit this model well enough. Note that the difference on the tools side isn't huge - instead of specifying a feature branch that should be merged (and perhaps also deleted) on success, we'd denote a specific hash to merge (just as in a pull request).

About credentials. For these long lived personal style branches, setup in Bamboo can be done once per developer, and giving plan editing (and perhaps plan admin) rights is enough, while the proposed solution as you've mentioned requires Bamboo admin rights including repository credentials, which while doable, pulls more safety from the system than we'd be comfortable with if given another choice. Also, our repository access scheme is secured, so only authorized developers are able to access and fork the repository, however it would still be useful to have the option to limit merge/pull requests to a smaller set of developers.

By the way, good thing you've reminded me about the rights issue. Have you considered making Bamboo admin access more fine grained? I.e., enhanced mainline repository credentials should not be given to developers, and they should not be able to trick the system set up new plans to merge/push to devel, however they should be allowed to create new plans for their own areas. Also it would be nice if they could refer to their own branches by selecting from project-wide source repositories that were set up by the admin. This way, they would not need to specify their own push/pull credentials for their own plans.

About the second concept. Basically I'm saying to offer an extra option for branch integration, like conditional branch integration that stage could use. Bamboo could do all regular integration chores as you've described, but no push would be made until a programmer lead reviews the changeset and clicks a button to accept.

[PiotrA]

Yeah, I see your points. Indeed there will be a problem with mainline credentials being exposed/cloneable to the developers and I don't think the current 4.0 design could be bended easily to support such forkflow. However I'll point this discussion up to the Bamboo product managers as I think they could be interested. I'm pretty sure we think about admin vs developers rights from time to time but these issues aren't of top priority (AFAIK) - for example there exists BAM-10614 issue already for refering their own branches from selecting Bamboo-wide source repositories set up by the admin.

As for the conditional branch integration - I feel in guts this feature will surface sooner or later (and no, the currently developed 4.0 has no conditional/manual push implemented yet), on the other hand I'm not sure how such deffered push should work in details. For example the Bamboo do all regular chores (and do the merge if neccesary), and later the lead programmer hit 'accept' - but I imagine there could be other changesets pushed in the meanwhile, so Bamboo would have to perform the merge/pull-request once again from scratch, wouldn't it? Anyway I have filed up a BAM-11172 for that feature.

[Thomas Nagy]

Thanks for your update.

Conditional branch integration in the general case could indeed bring up the problem you've described. However, remember that in the scenario outlined above it would only be applied to stage. Stage has the property that only the following two modifications can be made to it:

1. Bugfixes via conditional branch integration - bugfixes should generally not depend on or conflict with each other. Anyway, as reviewing these small changes doesn't take much effort and keeping stage running is of high importance, it should be done in near-real time many times a day. Though, it would be nice if you could give support for reevaluating outstanding conditional branch integration requests when the underlying branch (i.e., stage) has been pushed to.
2. Automatic merge from devel when instructing Bamboo to do a release - should be done atomically after merging all bugfixes, maybe a hypothetical Bamboo convenience feature would only allow to release if all pending bugfixes have been either accepted or rejected.

In both cases, only Bamboo has push rights to stage.

I have to reevaluate our strategy, but I think we will move away from Github to dedicated hard-coded merger Bamboo plans for now.

[PiotrA]

FYI:

While I confirmed we won't be supporting in 4.0 the "gated checkins via repository forks/pullRequests" we still would like to revisit it in a future release. So I've raised a JAC issue for that: https://jira.atlassian.com/browse/BAM-11205

regards,

PS

[M Vijay Kumar]

The developer forks the mainline repository, and in Bamboo the developer create a PlanBranch for the mainline Plan - that PlanBranch inherits all the mainline Plan's configuration, however the developer is able to override source repository - he/she would input there his/her forked repository url. This assumes the developer have some admin privileges on Bamboo, at least for creating mainline PlanBranches.

The developer works, pushes to his/her repo, Bamboo tests the code - when the developer is ready to merge his work into mainline he would enable 'branch integration' in his/her PlanBranch configuration, specyfing to merge with mainline and enabling "push on successful build". And the Bamboo should take care of the rest and refuse to push if something goes wrong during the merge. The only thing I'm not sure at this moment is whether the merge&push will work smoothly when doing the work on the same branch - 4.0 is designed assuming the developers are working on personal 'feature branches' not personal 'feature forked-repos'.

----------------------------------------------------------------------------------------------------------------------------------

Assuming the above approach, if the team of developers is huge how would this work? each developer forking the repository on the server, does this have any side affects.

[Kris Nuttycombe]

Related: https://answers.atlassian.com/questions/188883/building-pull-requests-from-stash-on-bamboo