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: rewrite how image env is merged #828

Merged
merged 3 commits into from
Sep 27, 2021

Conversation

catthehacker
Copy link
Member

@catthehacker catthehacker commented Sep 27, 2021

This is a replacement for #771 since i'm dumb the previous PR just adds environment to container and doesn't fix anything.

fixes #827
fixes #757 (for real this time)
fixes #161

@catthehacker catthehacker requested a review from a team as a code owner September 27, 2021 09:19
@mergify

This comment has been minimized.

@mergify mergify bot added the needs-work Extra attention is needed label Sep 27, 2021
@mergify

This comment has been minimized.

@catthehacker catthehacker linked an issue Sep 27, 2021 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Sep 27, 2021

Codecov Report

Merging #828 (efd1156) into master (0f04942) will increase coverage by 7.00%.
The diff coverage is 61.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #828      +/-   ##
==========================================
+ Coverage   49.27%   56.27%   +7.00%     
==========================================
  Files          23       23              
  Lines        2401     3730    +1329     
==========================================
+ Hits         1183     2099     +916     
- Misses       1090     1457     +367     
- Partials      128      174      +46     
Impacted Files Coverage Δ
pkg/container/docker_run.go 5.63% <16.16%> (+3.70%) ⬆️
pkg/common/git.go 49.82% <31.81%> (-9.97%) ⬇️
pkg/model/planner.go 49.78% <42.50%> (+16.70%) ⬆️
pkg/container/docker_build.go 59.72% <50.00%> (+59.72%) ⬆️
pkg/model/workflow.go 52.14% <63.63%> (+26.42%) ⬆️
pkg/container/docker_pull.go 36.36% <64.00%> (+18.18%) ⬆️
pkg/runner/runner.go 74.46% <70.37%> (-2.01%) ⬇️
pkg/runner/expression.go 85.71% <72.09%> (-0.93%) ⬇️
pkg/runner/step_context.go 78.57% <76.71%> (+9.62%) ⬆️
pkg/runner/run_context.go 81.88% <91.17%> (+5.48%) ⬆️
... and 25 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 7a426a0...efd1156. Read the comment docs.

@mergify mergify bot removed the needs-work Extra attention is needed label Sep 27, 2021
@cplee
Copy link
Contributor

cplee commented Sep 27, 2021

@catthehacker is this testable?

@catthehacker
Copy link
Member Author

definitely, will push test in few minutes

@catthehacker
Copy link
Member Author

there is still massive TODO to cross-test testdata between all packages and some other work, but I will explain that in github team discussions because it's too much typing :)

@mergify mergify bot merged commit 6c60af7 into nektos:master Sep 27, 2021
@catthehacker catthehacker deleted the cat/fix/proper-env-merge branch September 27, 2021 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants