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

internal/provisioner: Use local variable to replace the long access chain of fields #4586

Merged
merged 2 commits into from
Jun 22, 2022

Conversation

izturn
Copy link
Member

@izturn izturn commented Jun 18, 2022

it seems the fields of object nest too deeply, so use a local variable to replace it to make read code easy

Signed-off-by: Gang Liu gang.liu@daocloud.io

Signed-off-by: Gang Liu <gang.liu@daocloud.io>
@izturn izturn requested a review from a team as a code owner June 18, 2022 01:24
@izturn izturn requested review from skriss and sunjayBhatia and removed request for a team June 18, 2022 01:24
Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

The change itself looks fine to me, could you please run gofmt -s and check that make lint succeeds?

Also, this needs a changelog file.

Signed-off-by: Gang Liu <gang.liu@daocloud.io>
@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #4586 (b2ada19) into main (45ff8f7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4586      +/-   ##
==========================================
+ Coverage   76.82%   76.84%   +0.01%     
==========================================
  Files         140      140              
  Lines       12813    12818       +5     
==========================================
+ Hits         9844     9850       +6     
+ Misses       2712     2711       -1     
  Partials      257      257              
Impacted Files Coverage Δ
internal/provisioner/objects/service/service.go 73.42% <100.00%> (+0.65%) ⬆️
internal/sorter/sorter.go 98.78% <0.00%> (+0.60%) ⬆️

@izturn
Copy link
Member Author

izturn commented Jun 21, 2022

thx @youngnick done, but how can i set the release note label?

@tsaarni tsaarni added the release-note/minor A minor change that needs about a paragraph of explanation in the release notes. label Jun 21, 2022
Copy link
Member

@youngnick youngnick left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @izturn!

@skriss skriss merged commit 23a488b into projectcontour:main Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor A minor change that needs about a paragraph of explanation in the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants