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: allow existing logger from context #898

Merged
merged 1 commit into from
Nov 27, 2021
Merged

Conversation

ZauberNerd
Copy link
Contributor

We should reuse an existing context logger if in test context.
This will allow test to setup act with a null logger to assert
log messages.

@ZauberNerd ZauberNerd requested a review from a team as a code owner November 22, 2021 17:11
@codecov
Copy link

codecov bot commented Nov 22, 2021

Codecov Report

Merging #898 (be91b48) into master (0f04942) will increase coverage by 6.86%.
The diff coverage is 60.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #898      +/-   ##
==========================================
+ Coverage   49.27%   56.13%   +6.86%     
==========================================
  Files          23       26       +3     
  Lines        2401     3962    +1561     
==========================================
+ Hits         1183     2224    +1041     
- Misses       1090     1536     +446     
- Partials      128      202      +74     
Impacted Files Coverage Δ
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% <ø> (ø)
pkg/container/docker_run.go 5.53% <14.54%> (+3.60%) ⬆️
pkg/common/git.go 49.82% <31.81%> (-9.97%) ⬇️
pkg/runner/logger.go 65.43% <37.50%> (+1.28%) ⬆️
pkg/model/planner.go 49.78% <42.50%> (+16.70%) ⬆️
pkg/container/docker_build.go 58.97% <50.00%> (+58.97%) ⬆️
pkg/container/docker_pull.go 28.35% <58.06%> (+10.17%) ⬆️
... and 32 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 f726339...be91b48. Read the comment docs.

catthehacker
catthehacker previously approved these changes Nov 25, 2021
@mergify mergify bot requested a review from a team November 25, 2021 20:09
cplee
cplee previously approved these changes Nov 26, 2021
We should reuse an existing context logger if in test context.
This will allow test to setup act with a null logger to assert
log messages.

Co-authored-by: Markus Wolf <markus.wolf@new-work.se>
@ZauberNerd
Copy link
Contributor Author

@catthehacker @cplee had to rebase against master, because "Allow maintainer edits" doesn't work for organization accounts: isaacs/github#1681
The next PRs from @KnisterPeter and me will come from our personal forks, to make it easier on the mergify bot.

@mergify mergify bot requested a review from a team November 26, 2021 12:30
@mergify mergify bot requested a review from a team November 26, 2021 14:29
@mergify mergify bot merged commit 6517d04 into nektos:master Nov 27, 2021
@ZauberNerd ZauberNerd deleted the reuse-logger branch November 27, 2021 23:19
ZauberNerd added a commit to xing/act that referenced this pull request Nov 30, 2021
* nektos#898
* nektos#891

have both been merged upstream, so they're no longer required here.
ZauberNerd added a commit to xing/act that referenced this pull request Nov 30, 2021
* nektos#898
* nektos#891

have both been merged upstream, so they're no longer required here.
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