Skip to content
This repository was archived by the owner on Jul 11, 2023. It is now read-only.

fix: update configClient call and logging #4854

Merged
merged 2 commits into from
Jun 28, 2022

Conversation

jaellio
Copy link
Contributor

@jaellio jaellio commented Jun 28, 2022

Description:
Fixes incorrect configClient call and removes log definition.

Testing done:

  • CI

Fixes incorrect configClient call and removes log definition.

Signed-off-by: jaellio <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Merging #4854 (8ce0c4b) into main (4a3d57d) will increase coverage by 0.17%.
The diff coverage is 92.00%.

@@            Coverage Diff             @@
##             main    #4854      +/-   ##
==========================================
+ Coverage   69.34%   69.51%   +0.17%     
==========================================
  Files         219      219              
  Lines       16012    16033      +21     
==========================================
+ Hits        11104    11146      +42     
+ Misses       4854     4833      -21     
  Partials       54       54              
Flag Coverage Δ
unittests 69.51% <92.00%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/certificate/providers/types.go 0.00% <ø> (ø)
pkg/certificate/providers/config.go 79.23% <90.47%> (+10.94%) ⬆️
pkg/certificate/providers/options.go 100.00% <100.00%> (ø)
pkg/ticker/ticker.go 83.33% <0.00%> (-3.85%) ⬇️
pkg/certificate/manager.go 76.70% <0.00%> (-0.81%) ⬇️
pkg/certificate/providers/mrc.go 36.36% <0.00%> (+36.36%) ⬆️

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 baff85f...8ce0c4b. Read the comment docs.

)

var log = logger.New("certificate/provider")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was seeing lint errors here baff85f

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, in that case, we should remove (or alias) the import and keep this line; this assignment adds good context to the log lines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! Fixed!

@jaellio jaellio merged commit d970b24 into openservicemesh:main Jun 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants