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

feat: try to read ref and sha from event payload if available #889

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

ZauberNerd
Copy link
Contributor

With this change act will try to populate the githubContext.ref and
githubContext.sha with values read from the event payload.

Caveats:

  • page_build should not have a ref
  • status should not have a ref
  • registry_package should set the ref to the branch/tag but the
    payload isn't documented
  • workflow_call should set ref to the same value as its caller but the
    payload isn't documented
  • most of the events should set the sha to the last commit on the ref
    but unfortunately the sha is not always included in the payload,
    therefore we use the sha from the local git checkout

@codecov
Copy link

codecov bot commented Nov 17, 2021

Codecov Report

Merging #889 (18c3c61) into master (0f04942) will increase coverage by 8.16%.
The diff coverage is 65.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #889      +/-   ##
==========================================
+ Coverage   49.27%   57.43%   +8.16%     
==========================================
  Files          23       30       +7     
  Lines        2401     4269    +1868     
==========================================
+ Hits         1183     2452    +1269     
- Misses       1090     1611     +521     
- Partials      128      206      +78     
Impacted Files Coverage Δ
pkg/common/executor.go 46.90% <0.00%> (+2.03%) ⬆️
pkg/common/job_error.go 0.00% <0.00%> (ø)
pkg/common/outbound_ip.go 0.00% <0.00%> (ø)
pkg/common/testflag.go 0.00% <0.00%> (ø)
pkg/container/docker_volume.go 0.00% <0.00%> (ø)
pkg/model/action.go 0.00% <0.00%> (ø)
pkg/model/step_result.go 0.00% <0.00%> (ø)
pkg/container/docker_run.go 5.54% <14.15%> (+3.61%) ⬆️
pkg/common/git.go 49.82% <31.81%> (-9.97%) ⬇️
pkg/runner/logger.go 65.43% <37.50%> (+1.28%) ⬆️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e6cddf...18c3c61. Read the comment docs.

@ZauberNerd ZauberNerd marked this pull request as ready for review November 18, 2021 15:51
@ZauberNerd ZauberNerd requested a review from a team as a code owner November 18, 2021 15:51
@ZauberNerd ZauberNerd force-pushed the set-ref-sha branch 3 times, most recently from 38ca949 to a500d90 Compare November 27, 2021 18:37
@catthehacker
Copy link
Member

Could you add some test cases?

@mergify

This comment has been minimized.

@mergify mergify bot added the needs-work Extra attention is needed label Jan 3, 2022
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jan 3, 2022
@mergify mergify bot removed the needs-work Extra attention is needed label Jan 3, 2022
@mergify mergify bot requested a review from a team January 3, 2022 19:37
Copy link
Member

@catthehacker catthehacker left a comment

Choose a reason for hiding this comment

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

Still missing test cases

@mergify mergify bot requested a review from a team January 3, 2022 19:38
@ZauberNerd ZauberNerd marked this pull request as draft January 4, 2022 10:01
@ZauberNerd ZauberNerd marked this pull request as ready for review January 4, 2022 11:27
@mergify

This comment has been minimized.

@mergify mergify bot added the needs-work Extra attention is needed label Jan 4, 2022
With this change `act` will try to populate the `githubContext.ref` and
`githubContext.sha` with values read from the event payload.

Caveats:
- `page_build` should not have a ref
- `status` should not have a ref
- `registry_package` should set the ref to the branch/tag but the
  payload isn't documented
- `workflow_call` should set ref to the same value as its caller but the
  payload isn't documented
- most of the events should set the sha to the last commit on the ref
  but unfortunately the sha is not always included in the payload,
  therefore we use the sha from the local git checkout

Co-Authored-By: Philipp Hinrichsen <philipp.hinrichsen@new-work.se>
@mergify mergify bot removed the needs-work Extra attention is needed label Jan 4, 2022
@ZauberNerd
Copy link
Contributor Author

@catthehacker I added some tests, can you please take another look?

Copy link
Member

@catthehacker catthehacker left a comment

Choose a reason for hiding this comment

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

Looks more than "some" 😄 thanks a lot

@cplee cplee merged commit edd0fb9 into nektos:master Jan 21, 2022
@ZauberNerd ZauberNerd deleted the set-ref-sha branch February 10, 2022 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants