Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Take commit hash from CircleCI #1033

Merged
merged 2 commits into from
Apr 16, 2020
Merged

Conversation

valscion
Copy link
Contributor

@valscion valscion commented Apr 16, 2020

Similar to the support of Jenkins commit hash:

get commitHash(): string {
return this.env.GIT_COMMIT
}

describe("commit hash", () => {
it("returns correct commit hash when present", () => {
const env = {
...envs.ghprb,
GIT_COMMIT: "1234abc",
}
const jenkins = new Jenkins(env)
expect(jenkins.commitHash).toEqual("1234abc")
})
it("returns no commit hash when not present", () => {
const jenkins = new Jenkins(envs.ghprb)
expect(jenkins.commitHash).toBeUndefined()
})
})

Fixes the same problem as fixed in #905 but for CircleCI:

Danger doesn't currently have a way to understand on which commit is
running. To get the current commit it asks to the platform the list of
commits on the PR and assumes that is running on the last one. This
creates few problems when you push a new commit before Danger runs on
the CI for the previous commit, because it will assume is running on
the last one, but that is actually not true, and that means that both
the status update and the commit hash in the Danger comment signature
will be wrong.

From the CircleCI docs, CIRCLE_SHA1 should be the correct environment variable: https://circleci.com/docs/2.0/env-vars/#built-in-environment-variables

CIRCLE_SHA1String — The SHA1 hash of the last commit of the current build.

Similar to the support of Jenkins commit hash. Fixes the same problem as
fixed in 73d1439:

> Danger doesn't currently have a way to understand on which commit is
> running. To get the current commit it asks to the platform the list of
> commits on the PR and assumes that is running on the last one. This
> creates few problems when you push a new commit before Danger runs on
> the CI for the previous commit, because it will assume is running on
> the last one, but that is actually not true, and that means that both
> the status update and the commit hash in the Danger comment signature
> will be wrong.
@danger-public
Copy link

Warnings
⚠️ These providers are missing from the README: bamboo

Generated by 🚫 dangerJS against bf3e021

@orta
Copy link
Member

orta commented Apr 16, 2020

Cool, this works for me - thanks!

@orta orta merged commit 8090b86 into danger:master Apr 16, 2020
@peril-staging
Copy link
Contributor

peril-staging bot commented Apr 16, 2020

Thanks for the PR @valscion.

This PR has been shipped in v10.1.1 - CHANGELOG.

@valscion valscion deleted the circle-ci-commit-hash branch April 16, 2020 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants