Fix filtering deprecated charts #3635
Conversation
- Always show charts, just disable the option to select them in the details view - Optimize the loading of repo charts to sort and normalize once - Using the structured clone algorithm instead of JSON and fix the cache placement Signed-off-by: Sebastian Malton <[email protected]>
Nokel81
requested review from
Nachasic and
jim-docker
and removed request for
a team
Signed-off-by: Sebastian Malton <[email protected]>
There was a problem hiding this comment.
Left few minor comments. While testing works okay, fixes the issues listed in #3515. However, unable to find any deprecated chart versions to test this one (versions with line-through visibility) https://github.com/lensapp/lens/pull/3635/files#diff-4ada9d82cde72ab9fa11079e8dd1c571c9d3de88a5a5ff0494ac8c04eea92a25
|
|
||
| export class HelmChartManager { | ||
| protected cache: any = {}; | ||
| protected repo: HelmRepo; | ||
| static #cache = new Map<string, Buffer>(); |
There was a problem hiding this comment.
We're not using such style anywhere else. Probably better to omit the hash sign?
There was a problem hiding this comment.
This is a better version of private as it is actually enforced at runtime
There was a problem hiding this comment.
Why is this better? And why mark static property as private? Asking because static property can not be called on instance of a class.
There was a problem hiding this comment.
Same reason to mark an instance property as private, so to better model the encapsulation.
A static property that is # (JS private) cannot be accessed by code that is not a static member or an instance member.
Sure, you can add the |
Signed-off-by: Sebastian Malton <[email protected]>
| iter.map( | ||
| charts, | ||
| (chart => { | ||
| const __version = coerce(chart.version, { includePrerelease: true, loose: true }); |
There was a problem hiding this comment.
What is the meaning to use underscores before variable name here? __version?
There was a problem hiding this comment.
I meant it as both "temporary" and less likely to conflict with other fields. I chose __version because it is related to the version field but I don't want to override that field.
Signed-off-by: Sebastian Malton <[email protected]>
jakolehm
requested review from
panuhorsmalahti and
chenhunghan
and removed request for
Nachasic,
panuhorsmalahti and
chenhunghan
Always show charts, just disable the option to select them in the
details view
Optimize the loading of repo charts to sort and normalize once
Using the structured clone algorithm instead of JSON and fix the cache
placement
Signed-off-by: Sebastian Malton [email protected]
fixes #3515