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 ImageOS format #761

Merged
merged 2 commits into from
Aug 1, 2021
Merged

Fix ImageOS format #761

merged 2 commits into from
Aug 1, 2021

Conversation

bryannaegele
Copy link
Contributor

The current algorithm given ubuntu-18.04 returns ubuntu18.04 when it should be ubuntu18 according to actions/runner-images#345 (comment).

The current algorithm given `ubuntu-18.04` returns `ubuntu18.04` when it should be `ubuntu18` according to actions/runner-images#345 (comment).
@codecov
Copy link

codecov bot commented Jul 26, 2021

Codecov Report

Merging #761 (42832bf) into master (0f04942) will increase coverage by 1.34%.
The diff coverage is 57.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #761      +/-   ##
==========================================
+ Coverage   49.27%   50.61%   +1.34%     
==========================================
  Files          23       23              
  Lines        2401     2594     +193     
==========================================
+ Hits         1183     1313     +130     
- Misses       1090     1142      +52     
- Partials      128      139      +11     
Impacted Files Coverage Δ
pkg/container/docker_run.go 1.84% <0.00%> (-0.09%) ⬇️
pkg/model/workflow.go 25.29% <2.43%> (-0.43%) ⬇️
pkg/common/git.go 52.85% <30.15%> (-6.94%) ⬇️
pkg/runner/runner.go 68.96% <33.33%> (-7.51%) ⬇️
pkg/model/planner.go 34.56% <41.37%> (+1.48%) ⬆️
pkg/container/docker_pull.go 36.17% <64.70%> (+17.98%) ⬆️
pkg/runner/step_context.go 74.76% <75.81%> (+5.81%) ⬆️
pkg/runner/run_context.go 80.36% <98.55%> (+3.95%) ⬆️
pkg/runner/command.go 90.58% <100.00%> (+2.35%) ⬆️
pkg/runner/expression.go 87.91% <100.00%> (+1.27%) ⬆️
... and 5 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 4036b8a...42832bf. Read the comment docs.

@mergify
Copy link
Contributor

mergify bot commented Jul 26, 2021

@bryannaegele this pull request has failed checks 🛠

@mergify mergify bot added the needs-work Extra attention is needed label Jul 26, 2021
@bryannaegele
Copy link
Contributor Author

It seems all PRs are failing on the mac os test at the same spot. There's a warning about attempting to use deprecated short shas.

@catthehacker
Copy link
Member

catthehacker commented Jul 26, 2021

It seems all PRs are failing on the mac os test at the same spot. There's a warning about attempting to use deprecated short shas.

It fails at pulling Docker image. That is unfortunate thing we have to deal with since macOS runners don't have Docker pre-installed and thus GitHub doesn't have special API keys on them that have higher rate limits.

[basic/build] toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit
time="2021-07-26T06:36:22Z" level=debug msg="toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit"
time="2021-07-26T06:36:22Z" level=debug msg="toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit"
time="2021-07-26T06:36:22Z" level=debug msg="toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit"
time="2021-07-26T06:36:22Z" level=debug msg="toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit"
time="2021-07-26T06:36:23Z" level=debug msg="toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit"
[basic/build]   ❌  Failure - ./actions/action1
time="2021-07-26T06:36:23Z" level=debug msg="toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit"
time="2021-07-26T06:36:23Z" level=debug msg="toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: docker.com/increase-rate-limit"

@mergify mergify bot removed the needs-work Extra attention is needed label Jul 26, 2021
@bryannaegele
Copy link
Contributor Author

Ah. Gotcha. That's unfortunate.

If I need to add any test coverage, I'd be happy to. I'm a complete amateur at Go and didn't see a good place to add it since the original PR didn't have any coverage for this, so I'm at a bit of a loss on how to go about it though 😆

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.

Can be added in #750 otherwise TestGetGitHubContext would need to be rewritten to read full yaml or you would have to deal with yaml.Nodes which is hell

@mergify mergify bot requested a review from a team July 26, 2021 16:29
@bryannaegele
Copy link
Contributor Author

Sounds like adding it to #750 after merge makes the most sense.

@mergify mergify bot merged commit 531ea02 into nektos:master Aug 1, 2021
@bryannaegele bryannaegele deleted the patch-1 branch August 2, 2021 14:09
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.

3 participants