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

fix(semaphore): correct ENV variables #56

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

jaredmoody
Copy link
Contributor

@jaredmoody jaredmoody commented Apr 10, 2023

and add additional options from available variables

Per semaphore docs, use the correct ENV variable names.

This partially addresses #55, in that the reporter should now read the correct environment variables on Semaphore, but does not address --service-job-id being ignored when --done is present.

The only thing I'm not sure about here is the service_build_url and service_job_url semantics mapping. On semaphore, the "job" is the individual process and the "workflow" is the entire build.

@coveralls
Copy link
Member

Pull Request Test Coverage Report for Build 4660313599

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 90.402%

Totals Coverage Status
Change from base Build 4656075024: 0.05%
Covered Lines: 697
Relevant Lines: 771

💛 - Coveralls

Copy link
Contributor

@mrexox mrexox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey! Thank you for the PR. This looks great! I just want to reference Workflow in service_number, not in service_job_id. That's how it works for Github for example.

src/coverage_reporter/ci/semaphore.cr Outdated Show resolved Hide resolved
spec/coverage_reporter/config_spec.cr Outdated Show resolved Hide resolved
and add additional options from available variables

Co-Authored-By: Valentin Kiselev <[email protected]>
@jaredmoody
Copy link
Contributor Author

Great, made those updates.

@jaredmoody jaredmoody requested a review from mrexox April 11, 2023 18:01
@mrexox mrexox changed the title Semaphore: Correct ENV variables fix(semaphore): correct ENV variables Apr 12, 2023
@mrexox mrexox merged commit ffa4cf1 into coverallsapp:master Apr 12, 2023
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.

3 participants