search

reconquest / atlassian-external-hooks

External Hooks plugin for Atlassian Bitbucket
https://external-hooks.reconquest.io
Other
44 stars 37 forks source link

Add Merge Check capability #32

Closed greened closed 8 years ago

greened commented 8 years ago

Begin support for the Merge Check hook. Since a pull request has tons of information that Merge Check can examine and it's not clear what will ultimately be useful, begin by simply providing the to/from repositories and hashes.

Hooks will read from repository, from commit ID, to repository, to commit ID from stdin in that order. Any output on stdout will be reported as the merge failure message.

greened commented 8 years ago

Tested with stash 3.11.1.

andreineculau commented 8 years ago

Once again thanks!

For a detailed list of PR variables, which I'd very much like access to, yes all (no, not joking), here's the variables that pull-request-notifier-for-bitbucket exposes. A simple "PULL_REQUEST__" to "STASH_PR__" conversion would do, naming wise.

greened commented 8 years ago

Eventually it would be nice to pass all the information. However, I'm klnd of working in emergency mode to just get something running for our needs. I won't have a ton of time to add more at the moment so feel free to take what's here and run with it!

andreineculau commented 8 years ago

I won't have a ton of time to add more at the moment so feel free to take what's here and run with it!

Don't fret! Take it easy YOLO :) Thanks once again!

andreineculau commented 8 years ago

This is my work on top of yours: https://github.com/greened/atlassian-external-hooks/compare/merge-check...andreineculau:merge-check -- made it work with 4.3.2 (Bitbucket Server)

but some stuff should be of interest to you as well -- like the stdin args and the nl2br commits

andreineculau commented 8 years ago

FYI added 2 more commits on my branch: wrap with a PRE tag, and replace space with non-breaking space

These commits are helpful when running the pre-receive hook as merge-request hook:

screen shot 2016-03-15 at 14 21 16
seletskiy commented 8 years ago

I agree with @andreineculau, that additional information should be passed through environment variables, not through stdin.

Also, I guess it will be better, if merge hook code will share major codebase with ExternalPreReceiveHook, as it's done in ExternalAsyncPostReceiveHook.

greened commented 8 years ago

Back at this. This is all good feedback and I'll incorporate changes.

While we get this into shape, our IT guys want to know if this functionality (if not this exact design) will eventually be accepted into master/the main codebase. They just want assurance they won't be maintaining a special forked version forever.

greened commented 8 years ago

Andrei, thank you for your branch. It looks cleaner than mine. The only thing is that I need something that works with 3.11.1. I'm very new to the Stash/BitBucket API. Is it even possible to write a plugin that works with both? Requiring a rebuild for different versions is fine.

Actually, I see you have two branches: merge-check which is built on top of my original commit and merge-check-hook which appears to be a separate history. Which one do you prefer to use going forward?

greened commented 8 years ago

We've decided to go with Andrei's code for this.