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: vela addon upgrade missing the history args #4388

Merged
merged 1 commit into from
Jul 22, 2022

Conversation

StevenLeiZhang
Copy link
Collaborator

@StevenLeiZhang StevenLeiZhang commented Jul 15, 2022

Signed-off-by: StevenLeiZhang zhangleiic@163.com

Description of your changes

Fixes #4311
How to fix it:
Merge legacy parameters and new input parameters.

  1. if legacy parameters do not exist, use new parameters
  2. if legacy parameters exists, merge them and new parameter overwrite the legacy one

I have:

  • Read and followed KubeVela's contribution process.
  • Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
  • Run make reviewable to ensure this PR is ready for review.
  • Added backport release-x.y labels to auto-backport this PR if necessary.

How has this code been tested

Special notes for your reviewer

@codecov
Copy link

codecov bot commented Jul 15, 2022

Codecov Report

Merging #4388 (276984a) into master (957302c) will increase coverage by 0.05%.
The diff coverage is 52.38%.

@@            Coverage Diff             @@
##           master    #4388      +/-   ##
==========================================
+ Coverage   61.45%   61.50%   +0.05%     
==========================================
  Files         347      347              
  Lines       34260    34281      +21     
==========================================
+ Hits        21054    21086      +32     
+ Misses      10454    10444      -10     
+ Partials     2752     2751       -1     
Flag Coverage Δ
apiserver-e2etests 27.14% <0.00%> (+0.04%) ⬆️
apiserver-unittests 40.06% <ø> (-0.02%) ⬇️
core-unittests 56.46% <52.38%> (-0.04%) ⬇️
e2e-multicluster-test 19.78% <0.00%> (-0.02%) ⬇️
e2e-rollout-tests 22.26% <ø> (ø)
e2etests 28.73% <ø> (+0.06%) ⬆️

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

Impacted Files Coverage Δ
pkg/addon/addon.go 64.11% <52.38%> (-0.32%) ⬇️
...tepdefinition/workflowstepdefinition_controller.go 69.14% <0.00%> (-7.45%) ⬇️
...aits/traitdefinition/traitdefinition_controller.go 70.52% <0.00%> (-2.11%) ⬇️
pkg/addon/registry.go 53.01% <0.00%> (ø)
pkg/apiserver/event/sync/worker.go 77.08% <0.00%> (ø)
.../core/scopes/healthscope/healthscope_controller.go 72.17% <0.00%> (+0.28%) ⬆️
pkg/apiserver/domain/service/addon.go 59.42% <0.00%> (+0.72%) ⬆️
...ller/core.oam.dev/v1alpha2/application/revision.go 73.30% <0.00%> (+0.91%) ⬆️
pkg/apiserver/event/sync/cr2ux.go 41.79% <0.00%> (+1.49%) ⬆️
pkg/addon/push.go 76.80% <0.00%> (+1.59%) ⬆️
... and 5 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 957302c...276984a. Read the comment docs.

pkg/addon/addon.go Outdated Show resolved Hide resolved
@charlie0129
Copy link
Member

Great, please let the user know that the arguments will be merged in the command help text.

Copy link
Collaborator

@wonderflow wonderflow left a comment

Choose a reason for hiding this comment

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

great job

Signed-off-by: StevenLeiZhang <zhangleiic@163.com>
@StevenLeiZhang
Copy link
Collaborator Author

Great, please let the user know that the arguments will be merged in the command help text.

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vela addon upgrade missing the history args
4 participants