feat(Helm-Chart): Groups same helm chart into single tile #5699
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
abhinandan13jan
force-pushed
the
helm-group
branch
from
5616dea
to
5acdb6c
Compare
abhinandan13jan
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
| }; | ||
| const existingChart = normalizedCharts.findIndex((hlc) => hlc.obj?.name === chartName); | ||
| if (existingChart > -1) { | ||
| if (normalizedCharts[existingChart].obj?.version < helmChart.obj.version) { |
There was a problem hiding this comment.
@rohitkrai03 Should this string comp be fine?
There was a problem hiding this comment.
Probably better to use natural sort here for cases like 1.10.0 > 1.2.0. You can use String,prototype.localeCompare() with numeric option. Here's an example. It returns 1, meaning 1.10.0 goes after 1.2.0:
'1.10.0'.localeCompare('1.2.0', undefined, {numeric: true, sensitivity: 'base'});
| }; | ||
| const existingChart = normalizedCharts.findIndex((hlc) => hlc.obj?.name === chartName); | ||
| if (existingChart > -1) { | ||
| if (normalizedCharts[existingChart].obj?.version < helmChart.obj.version) { |
There was a problem hiding this comment.
Probably better to use natural sort here for cases like 1.10.0 > 1.2.0. You can use String,prototype.localeCompare() with numeric option. Here's an example. It returns 1, meaning 1.10.0 goes after 1.2.0:
'1.10.0'.localeCompare('1.2.0', undefined, {numeric: true, sensitivity: 'base'});
abhinandan13jan
force-pushed
the
helm-group
branch
from
5acdb6c
to
799c6ec
Compare
| @@ -19,6 +19,8 @@ type Metadata = { uid?: string; name?: string; namespace?: string }; | |||
|
|
|||
| export type Item = { | |||
| obj?: { | |||
| name?: string; | |||
There was a problem hiding this comment.
You don't need this change now.
| (hlc) => hlc.obj?.name === chartName, | ||
| ); | ||
| if (existingChartIndex > -1) { | ||
| const versionCompare = helmChart.obj.version.localeCompare( |
There was a problem hiding this comment.
| const versionCompare = helmChart.obj.version.localeCompare( | |
| const versionCompare = helmChart.obj?.version?.localeCompare( |
abhinandan13jan
force-pushed
the
helm-group
branch
from
799c6ec
to
a9b2770
Compare
abhinandan13jan
force-pushed
the
helm-group
branch
from
a9b2770
to
a5ec35c
Compare
|
verified this locally, works fine |
|
/assign @christianvogt |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinandan13jan, christianvogt, debsmita1, rohitkrai03 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Fixes:
https://issues.redhat.com/browse/ODC-4085
Analysis / Root cause:
Group helm multiple chart versions together in developer catalog
Solution Description:
Altered normalizeHelm functionality to create single item for all hellm chart versions with same name
Screenshot
After change
Test
Added to catalog test suite
Browser conformation
Chrome 73