Skip to content

Fix duplicate versions for same chart#492

Merged
scbizu merged 2 commits into
helm:mainfrom
ninjadq:fix_duplicate_versions
Sep 30, 2021
Merged

Fix duplicate versions for same chart#492
scbizu merged 2 commits into
helm:mainfrom
ninjadq:fix_duplicate_versions

Conversation

@ninjadq

@ninjadq ninjadq commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

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

scbizu commented Sep 25, 2021

Copy link
Copy Markdown
Contributor

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

ninjadq commented Sep 25, 2021

Copy link
Copy Markdown
Contributor Author

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

Comment thread pkg/chartmuseum/server/multitenant/handlers.go
Comment thread pkg/chartmuseum/server/multitenant/handlers.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/handlers.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/handlers.go Outdated
Comment thread pkg/chartmuseum/server/multitenant/handlers.go
Comment thread pkg/chartmuseum/server/multitenant/handlers.go Outdated
@scbizu

scbizu commented Sep 25, 2021

Copy link
Copy Markdown
Contributor

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

scbizu commented Sep 30, 2021

Copy link
Copy Markdown
Contributor

LGTM 😎

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

scbizu commented Sep 30, 2021

Copy link
Copy Markdown
Contributor

Merging , the bad CI will be fixed in #482

@scbizu
scbizu merged commit 670c99e into helm:main Sep 30, 2021
@ninjadq

ninjadq commented Oct 1, 2021

Copy link
Copy Markdown
Contributor Author

Thank you @scbizu, happy holidays

@scbizu

scbizu commented Oct 1, 2021

Copy link
Copy Markdown
Contributor

@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