diff --git a/README.md b/README.md index 13f91e1..3e57ed0 100644 --- a/README.md +++ b/README.md @@ -1,38 +1,125 @@ # GitHub Pull Request Plugin -A [Buildkite plugin](https://buildkite.com/docs/agent/v3/plugins) to help build the GitHub post-merge check PR ref (`refs/pull/123/merge`). +A [Buildkite plugin](https://buildkite.com/docs/agent/v3/plugins) to build the merged state of GitHub pull requests. -This plugin helps you run CI run against the "merged" branch, helping catch bad merges. +Why would I want this? + 1. Parity with other CI setups. Including [TeamCity](https://blog.jetbrains.com/teamcity/2013/02/automatically-building-pull-requests-from-github-with-teamcity/), Travis, and [Jenkins](https://wiki.jenkins.io/display/JENKINS/GitHub+pull+request+builder+plugin). + 2. Can help avoid building stale branches, as CI is done against the target branch at the time of the PR being raised rather than when the source branch was created + 3. Reduce the risk of failed builds from reaching master, due to bad merges (See the example at the bottom of this document) + +# Modes The plugin has two modes: - `mode: trigger` to async trigger another build (of the current pipeline); and - `mode: checkout` to checkout the PR merged ref (`refs/pull/123/merge`) rather than the head (`refs/pull/123/head`) -## Example +# Example + +## Checkout Mode +When using `checkout` mode, the plugin should be specified on each build step (e.g. where a checkout happens)he ```yml +plugins: &plugins + seek-oss/github-merged-pr#v0.0.5: + mode: checkout + steps: - - plugins: - seek-oss/github-merged-pr#v0.0.5: - mode: checkout + - label: 'Make something' + commands: + - make something + plugins: + <<: *plugins + + - label: 'Make something else' + commands: + - make something-else + plugins: + <<: *plugins ``` +## Trigger Mode +In `trigger` mode the plugin should only be specified on one step, to prevent triggering multiple builds. + ```yml steps: - - plugins: - seek-oss/github-merged-pr#v0.0.5: - mode: trigger + - label: 'Make something' + commands: + - make something + seek-oss/github-merged-pr#v0.0.5: + mode: trigger + + - label: 'Make something else' + commands: + - make something-else ``` Ensure `Skip pull request builds for existing commits` is set to `false` in your Pipeline settings, as BuildKite will build the branch and skip the PR build. -## Tests +# Tests To run the tests of this plugin, run ```sh docker-compose run --rm tests ``` -## License +# License MIT (see [LICENSE](LICENSE)) + +# Bad Merge Example + +An example of why it may be desirable to build the merged commit. Let's say you're raising a PR to implement a `subtract` function to a calculator that already does `add`ition: + +`origin/master` (commit `1abcdef`): calculator.js +``` +class Calculator { + constructor() { + this.currentAnswer = 0; + } + + subtract(value) { + this.currentAnswer -= value; + } +} +``` + +(Tests omitted for brevity). + +You create a branch locally to implement your feature + +`feature/add` (commit `feba123`) (parent commit `1abcdef`): calculator.js +``` +class Calculator { + constructor() { + this.currentAnswer = 0; + } + + subtract(value) { + this.currentAnswer -= value; + } + + add(value) { + this.currentAnswer += value; + } +} +``` + +In the meantime, `origin/master` is updated to rename `currentAnswer` to `answer`. +`origin/master` (commit `56789ab`) (parent commit `1abcdef`): calculator.js +``` +class Calculator { + constructor() { + this.answer = 0; + } + + subtract(value) { + this.answer -= value; + } +} +``` + +You then raise your PR for `feature/add`, but: + 1. CI will run against the `HEAD` of the branch (commit `feba123`); and + 2. The PR will likely be merged (as tests pass) -- unless someone on your team notices; and + 3. The `git merge` will succeed (as there's no text conflicts); and + 4. The target branch (`master`) will be broken :(