[Tooling] Add github_commit_status to all pipeline jobs
#115
Conversation
Generated by 🚫 Danger |
.buildkite/pipeline.yml
Outdated
|
|
||
| - label: ":swift: Standalone Swift Package - Explicit" | ||
| command: tests/install_swiftpm_dependencies/test_scripts/test_package_explicit.sh | ||
| notify: | ||
| - github_commit_status: | ||
| context: Standalone Swift Package - Explicit |
There was a problem hiding this comment.
I wonder if it would make sense to prefix the context of all of those with install_swiftpm_dependencies Tests: or [install_swiftpm_dependencies Tests], so that they make more sense when seen in as list of checks in a GitHub PR?
| context: Standalone Swift Package - Explicit | |
| context: install_swiftpm_dependencies Tests: Standalone Swift Package - Explicit |
| context: Standalone Swift Package - Explicit | |
| context: [install_swiftpm_dependencies Tests] Standalone Swift Package - Explicit |
There was a problem hiding this comment.
Let's see how it looks: updated on 6d4444f
There was a problem hiding this comment.
Looks nicely grouped:
There was a problem hiding this comment.
@AliSoftware @mokagio
I've deployed https://github.com/Automattic/buildkite-ci/pull/516, updated the required checks in this repo and ran CI again.
It looks good, but there's still a check for the entire pipeline, I believe, and that one uses a name based on the Buildkite slug it seems.
I guess this is represented in the setting publish_commit_status and shows up both in the commit history...
...and in the PR checks.
I think there's value in keeping this setting enabled, as it seems to represent the success / failure of the entire pipeline; perhaps, for this repo, that can be the only required check for this repo as opposed to individual checks.
Having the checks on the commits is also interesting (though often intermediate commits will have their builds cancelled if a newer commit is pushed, per our settings).
There was a problem hiding this comment.
I think there's value in keeping this setting enabled, as it seems to represent the success / failure of the entire pipeline; perhaps, for this repo, that can be the only required check for this repo as opposed to individual checks.
Sounds good to me in the context of this project. I guess that was the intention behind making it required when that setting was made (just a guess, I don't remember the early days of this repo).
Having the checks on the commits is also interesting (though often intermediate commits will have their builds cancelled if a newer commit is pushed, per our settings).
Unless I'm missing something, that is the default behavior in GitHub for every PR where you push commits after having opened the PR. Example:
I agree that it's interesting. The more info, the better. I've been in a handful of situations where having those checks in the GitHub UI was useful to quickly identify which commit broke the build.
There was a problem hiding this comment.
Agreed on all counts 👍
There was a problem hiding this comment.
I see @AliSoftware 's suggestion has been addressed and the result is neat.
The goal of this and related PRs is to have a reference implementation for paaHJt-78h-p2: pipeline status updates with explicit names, with Buildkite automatic status disabled.
This PR depends on https://github.com/Automattic/buildkite-ci/pull/516
CHANGELOG.mdif necessary.