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 duplicate versions for same chart #492

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

ninjadq
Copy link
Contributor

@ninjadq ninjadq commented Sep 24, 2021

Signed-off-by: DQ [email protected]

@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
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
* 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 <[email protected]>
@ninjadq ninjadq force-pushed the fix_duplicate_versions branch from c1a1020 to 5dd7f03 Compare September 26, 2021 03:32
* 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 <[email protected]>
@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

@ninjadq You too 🥰

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

Successfully merging this pull request may close these issues.

3 participants