lunny/helm-chart
1
0
Fork 1
Code Issues 36 Pull Requests 14 Actions Packages Projects Releases 93 Wiki Activity

Format all files with prettier VSCode plugin and add yamllint in CI #413

Merged
pat-s merged 10 commits from refs/pull/413/head into main 2023-03-28 21:18:23 +00:00
Conversation 11 Commits 10 Files Changed 8
+1113 -5424
pat-s commented 2023-03-09 16:32:47 +00:00 (Migrated from gitea.com)
Copy Link
Copy Source

@justusbunsi to end my formatting mess... ;)

I am not fully sure myself about the linebreaks in values.yaml but I don't think there's an easy way to change that behavior.

@justusbunsi to end my formatting mess... ;) I am not fully sure myself about the linebreaks in `values.yaml` but I don't think there's an easy way to change that behavior.
6543 commented 2023-03-21 02:29:35 +00:00 (Migrated from gitea.com)
Copy Link
Copy Source

@pat-s can we add a lint step to CI to ensure it stay like this?

@pat-s can we add a lint step to CI to ensure it stay like this?
pat-s commented 2023-03-21 09:00:19 +00:00 (Migrated from gitea.com)
Copy Link
Copy Source

Good idea! https://github.com/cytopia/docker-yamllint looks promising to me. Let's see if I can get it configured quickly.

Good idea! https://github.com/cytopia/docker-yamllint looks promising to me. Let's see if I can get it configured quickly.
justusbunsi (Migrated from gitea.com) reviewed 2023-03-21 09:15:29 +00:00
pat-s commented 2023-03-21 09:16:39 +00:00 (Migrated from gitea.com)
Copy Link
Copy Source

@6543 Not sure if it covers all cases but I don't have resources now to check for each edge case :)
I'd argue: "better than before" ?

@6543 Not sure if it covers all cases but I don't have resources now to check for each edge case :) I'd argue: "better than before" ?
justusbunsi (Migrated from gitea.com) approved these changes 2023-03-21 18:09:41 +00:00
justusbunsi (Migrated from gitea.com) left a comment
Copy Link
Copy Source

LGTM

LGTM
techknowlogick (Migrated from gitea.com) reviewed 2023-03-25 10:14:37 +00:00
justusbunsi (Migrated from gitea.com) reviewed 2023-03-28 19:25:17 +00:00
Chart.lock
@@ -13,3 +2,1 @@
version: 19.6.4
digest: sha256:462d513ac8ef7abfe26030fd2ea93eb79df167a861ebe09d6c58c7dcd5601e85
generated: "2024-11-30T00:41:29.178889496Z"
- name: memcached
justusbunsi (Migrated from gitea.com) commented 2023-03-28 19:25:17 +00:00
Copy Link
Copy Source

If possible, we should probably disable formatting autogenerated files like Chart.lock. A helm dependency update with actually updated dependency version will remove the indentation again.

If possible, we should probably disable formatting autogenerated files like _Chart.lock_. A `helm dependency update` with actually updated dependency version will remove the indentation again.
pat-s (Migrated from gitea.com) reviewed 2023-03-28 21:15:28 +00:00
Chart.lock
@@ -13,3 +2,1 @@
version: 19.6.4
digest: sha256:462d513ac8ef7abfe26030fd2ea93eb79df167a861ebe09d6c58c7dcd5601e85
generated: "2024-11-30T00:41:29.178889496Z"
- name: memcached
pat-s (Migrated from gitea.com) commented 2023-03-28 21:15:28 +00:00
Copy Link
Copy Source

I think it's fine: the formatting is only targeting the indentation (which is off imo and according to the formatter).
The indentation is not changed when running helm dependency update.

So I think once we get the PR in, there shouldn't be any conflicts in this regard.

I think it's fine: the formatting is only targeting the indentation (which is off imo and according to the formatter). The indentation is not changed when running `helm dependency update`. So I think once we get the PR in, there shouldn't be any conflicts in this regard.
pat-s commented 2023-03-28 21:17:54 +00:00 (Migrated from gitea.com)
Copy Link
Copy Source

@justusbunsi I'm gonna merge this so I don't have to rebase another dozen times :) If something is troubling us afterwards, we can still account/adapt.

@justusbunsi I'm gonna merge this so I don't have to rebase another dozen times :) If something is troubling us afterwards, we can still account/adapt.
justusbunsi commented 2023-03-28 21:23:54 +00:00 (Migrated from gitea.com)
Copy Link
Copy Source

Alright. 👍

Regarding the helm dependency update: it won't change indentation when nothing has changed in chart.yaml. If this file has a change on the dependency version, the update command will rewrite the lock file and resets the manually changed indentation. I've tested those two cases prior to mentioning it. But thinking about it: since changing the dep version requires rewriting the lock file anyway, it's nothing to be concerned about.

Alright. 👍 Regarding the `helm dependency update`: it won't change indentation when nothing has changed in chart.yaml. If this file has a change on the dependency version, the update command will rewrite the lock file and resets the manually changed indentation. I've tested those two cases prior to mentioning it. But thinking about it: since changing the dep version requires rewriting the lock file anyway, it's nothing to be concerned about.
pat-s commented 2023-03-29 07:06:49 +00:00 (Migrated from gitea.com)
Copy Link
Copy Source

Ah thanks, you're right, that's unfortunate (just tried it).

See #424 for a possible solution.

Ah thanks, you're right, that's unfortunate (just tried it). See #424 for a possible solution.
Sign in to join this conversation.
Reviewers
No Reviewers
justusbunsi
techknowlogick
Labels
Clear labels
has/backport
in progress
invalid
kind/breaking
kind/bug
kind/build
kind/dependency
kind/deployment
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/proposal
kind/question
kind/refactor
kind/security
kind/testing
kind/translation
kind/ui
need/backport
priority/critical
priority/low
priority/maybe
priority/medium
reviewed/duplicate
reviewed/invalid
reviewed/wontfix
skip-changelog
status/blocked
status/needs-feedback
status/needs-reviews
status/wip
upstream/gitea
upstream/other
No labels kind/lint
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
lunny (Lunny Xiao)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: lunny/helm-chart#413
Delete Branch "refs/pull/413/head"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?