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.

Sign up for GitHub

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

Jump to bottom

refactor to use util/bklog.G(ctx) instead logrus. directly #2236

Merged
merged 1 commit into from
Jul 13, 2021
Merged

refactor to use util/bklog.G(ctx) instead logrus. directly #2236

merged 1 commit into from
Jul 13, 2021

Conversation

morlay
Copy link
Contributor

@morlay morlay commented Jul 9, 2021 •
edited
Loading

Too may changes

I'm not sure, Should I add new param ctx context.Context, specially for public functions

@morlay morlay force-pushed the master branch 3 times, most recently from 200441f to a278098 Compare July 9, 2021 07:48
tonistiigi
tonistiigi reviewed Jul 10, 2021
View reviewed changes
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

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

Generally looks ok to me. Do I understand correctly that bklog.G(ctx) can be stored as a value and use multiple times to generate a log?

util/bklog/log.go Show resolved Hide resolved
control/control.go Outdated Show resolved Hide resolved
@morlay
Copy link
Contributor Author

morlay commented Jul 13, 2021

bklog.G(ctx) can be stored as a value and use multiple times to generate a log?

Yes, but could only as variable in a function scope

@morlay morlay marked this pull request as ready for review July 13, 2021 02:52
@morlay
Copy link
Contributor Author

morlay commented Jul 13, 2021 •
edited
Loading

@tonistiigi

  • removed all logrus. in each pkg (exclude test files).
  • added context.Context as first parameter if required.
  • use bklog.G(context.TODO()) for hard to refactor, like solver, may next PR to do more refactor.

tonistiigi
tonistiigi reviewed Jul 13, 2021
View reviewed changes
client/solve.go Outdated Show resolved Hide resolved
tonistiigi
tonistiigi reviewed Jul 13, 2021
View reviewed changes
worker/base/worker.go Outdated Show resolved Hide resolved
@morlay morlay force-pushed the master branch 6 times, most recently from ee22977 to b715f9a Compare July 13, 2021 03:38
@morlay
refactor to use util/bklog instead of using logurs directly
18b49fd 
Signed-off-by: Morlay <morlay.null@gmail.com>
@tonistiigi tonistiigi merged commit 06e8602 into moby:master Jul 13, 2021
@coryb coryb mentioned this pull request Mar 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonistiigi tonistiigi tonistiigi approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@morlay @tonistiigi @AkihiroSuda