Skip to content

Fix duplicate versions for same chart #492

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
Merged
scbizu merged 2 commits into from
Sep 30, 2021
Merged

Fix duplicate versions for same chart #492

scbizu merged 2 commits into from
Sep 30, 2021

Conversation

@ninjadq
Copy link
Contributor

@ninjadq ninjadq commented Sep 24, 2021

Signed-off-by: DQ dengq@vmware.com

@ninjadq ninjadq mentioned this pull request Sep 25, 2021
Closed
@ninjadq ninjadq force-pushed the fix_duplicate_versions branch 2 times, most recently from bd2d518 to ccc3675 Compare September 25, 2021 04:21
@scbizu
Copy link
Contributor

scbizu commented Sep 25, 2021

Hi @ninjadq , could you add some test cases for this PR ?

@ninjadq ninjadq force-pushed the fix_duplicate_versions branch from ccc3675 to fba104a Compare September 25, 2021 15:00
@ninjadq
Copy link
Contributor Author

ninjadq commented Sep 25, 2021

hi @scbizu , added a case similar to chart only scenario

scbizu
scbizu requested changes Sep 25, 2021
View reviewed changes
@scbizu
Copy link
Contributor

scbizu commented Sep 25, 2021

Added some review suggestions , thanks for your PR again

@ninjadq ninjadq force-pushed the fix_duplicate_versions branch from fba104a to c1a1020 Compare September 26, 2021 03:22
@ninjadq
Fix duplicate versions for same chart
5dd7f03 
* The detailed issue is described in helm#450
* And there is a PR helm#454 fixed one scenario of this issue
* But there is another ocassion in which users upload chart with prov
* in this PR is to handle this situation with the way similar with helm#454

Signed-off-by: DQ <dengq@vmware.com>
@ninjadq ninjadq force-pushed the fix_duplicate_versions branch from c1a1020 to 5dd7f03 Compare September 26, 2021 03:32
@ninjadq
Enhance: optimize loop in getChartAndProvFiles
1ecdaa5 
* If conflict, it didn't need to do the left logic, just return the file
* move out file format check logic out of `validateChartOrProv`
* these changes are discussed in helm#492 (comment)

Signed-off-by: DQ <dengq@vmware.com>
@scbizu
Copy link
Contributor

scbizu commented Sep 30, 2021

LGTM 😎

@scbizu scbizu added this to the v0.14.0 milestone Sep 30, 2021
@scbizu
Copy link
Contributor

scbizu commented Sep 30, 2021

Merging , the bad CI will be fixed in #482

@scbizu scbizu merged commit 670c99e into helm:main Sep 30, 2021
@ninjadq
Copy link
Contributor Author

ninjadq commented Oct 1, 2021

Thank you @scbizu, happy holidays

@scbizu
Copy link
Contributor

scbizu commented Oct 1, 2021 •
edited
Loading

@ninjadq You too 🥰

@reasonerjt reasonerjt mentioned this pull request Jan 21, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@scbizu scbizu scbizu approved these changes

Assignees

No one assigned

Labels

size/M

Projects

None yet

Milestone

v0.14.0

Development

Successfully merging this pull request may close these issues.

3 participants

@ninjadq @scbizu @helm-bot