komodorio / helm-dashboard

The missing UI for Helm - visualize your releases
Apache License 2.0
5k stars 302 forks source link

Added greater granularity for boolean environment variables #511

Closed alessandrodetta closed 9 months ago

alessandrodetta commented 9 months ago

Changes Proposed

See suggestion in #510.

I preferred to issue this pull request as draft because this is my first open source pull request.

I will suggest in the future anyway to add implementation for some kind of structure (maybe a map?) to store parsed environment variables for both boolean and strings (in already created env.go) so that the environment variables are parsed for their correct values in one single point in code and not sparsely in different files as it is now and also so that reading multiple times their values doesn't involve more calls to the OS.

Furthermore, somehow I would also suggest integrating the management of program options into this reasoning, in order to clearer management since in some cases there is a relationship between the two.

Check List

undera commented 9 months ago

As for CI failure, try merging from master, I have updated golang version to 1.21 there

alessandrodetta commented 9 months ago

@undera Probably you got notified, but I have updated the solution with the changes you reviewed. I have rebased the feature branch so that now is on top of the current main branch so that I don't need to re-create a PR. Your idea was to create a new PR with my changes from main itself or like this is ok?

codecov-commenter commented 9 months ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 30.18%. Comparing base (3d7f907) to head (4aeec65). Report is 24 commits behind head on main.

Files with missing lines Patch % Lines
pkg/dashboard/server.go 0.00% 2 Missing :warning:
pkg/dashboard/api.go 0.00% 0 Missing and 1 partial :warning:
pkg/dashboard/objects/data.go 0.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #511 +/- ## ========================================== + Coverage 29.72% 30.18% +0.46% ========================================== Files 10 10 Lines 1329 1335 +6 ========================================== + Hits 395 403 +8 + Misses 893 891 -2 Partials 41 41 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

undera commented 9 months ago

Please do another pull from master, there is small issue with static check setup in CI

undera commented 9 months ago

Awesome! Thanks!