fmjstudios / helm

🪖 A collection of MIT-licensed Helm Charts
MIT License
7 stars 10 forks source link

chore(charts/paperless-ngx): remove line breaks from comments #21

Closed fty4 closed 1 week ago

fty4 commented 1 month ago

What this PR does / why we need it

Just removes some whitespace / line breaks which will be generated from comments in charts/paperless-ngx/templates/configmap.yaml. Also updating helpers for same comment convention.

Which issue this PR fixes

no issue created

Special notes for your reviewer

no special note

Checklist

github-actions[bot] commented 4 weeks ago

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

fty4 commented 2 weeks ago

Please do not close PR - still valid.

FMJdev commented 1 week ago

Hey @fty4 , thank you for the changes again! This sure improves readability 💯 I'll merge right away once you've had time to add the version bump so we keep immutability 👍🏼

fty4 commented 1 week ago

Hello @FMJdev I've bumped the chart version - after this is merged I also will bump the version of #22 if this is fine for you!

I added some fix to complete ct lint (new linke, spaces) - but still struggling with the following:

 ✖︎ paperless-ngx => (version: "0.2.5", path: "charts/paperless-ngx") > failed validating maintainer "FMJ Studios Helm Authors": 404 Not Found

How would you link to fix that? If you want I can move the fix commits into another PR (I added theme here to get sucessful checks).

fty4 commented 1 week ago

It seems the name must match a GitHub username / organization.

diff --git a/charts/paperless-ngx/Chart.yaml b/charts/paperless-ngx/Chart.yaml
index cbc6df7..4123eed 100644
--- a/charts/paperless-ngx/Chart.yaml
+++ b/charts/paperless-ngx/Chart.yaml
@@ -36,7 +36,7 @@ keywords:
 sources:
   - "https://github.com/fmjstudios/helm"
 maintainers:
-  - name: FMJ Studios Helm Authors
+  - name: fmjstudios
     email: info@fmj.studio
   - name: FMJdev
     email: info@fmj.dev

When adding this change the linting will be sucessful.

see: helm/chart-testing#659

FMJdev commented 1 week ago

Hey, first off thank you for the research into the issue. Very surprising to see this pop up now. I wanted to use the Authors rather than the GitHub Org to credit everybody in the AUTHORS file, but nvm I guess.

I took a closer look at the issue and like you noted and commited ct will then start complaining about bracket spacing and missing ending newlines. I wanted to add pre-commit anyways to aid with consistency and took the time to erase all of these issues across all charts as well as add a new chart-testing target for make for more regular linting. Sorry for voiding your work somewhat there. I published 0.2.5 already as a result of this so I'd kindly ask you to edit the PR a final time 🤞🏼

Feel free to edit #22 once we have this sorted out. I'll merge when you've had the time. Thanks a lot once again! 🥇

fty4 commented 1 week ago

Solved the merge conflict and bumped the chart version.

Now creating the kind cluster in Testing failed:

docker: Error response from daemon: driver failed programming external connectivity on endpoint chart-testing-control-plane (00b7cdf4af30278583d36d498b18c9c4576dc29d2a88e921150f312a1e88bfd3): Error starting userland proxy: listen tcp4 0.0.0.0:22: bind: address already in use.

FMJdev commented 1 week ago

How fitting for all of this to pop up during your PR, not during my hundreds of commits 😆 The Testing workflow is quite finicky due to the changed checks 👈🏼 I'm going to forego CI here since it's a bogus issue and I'm going to take a closer look at it on the weekend. Seems like GitHub's CI machines already have port 22 bound, obviously most likely for SSH but I would've expected CI runners to use custom ports.

Nonetheless, thank you for all your work so far! 🥇

fty4 commented 1 week ago

😆 that was no problem for me at all - I am happy to see my changes merged instead of having to maintain my changes in the fork.

Thank you for sharing this chart - I think it is better than the others around here. Therefore I am happy to be able to use this chart! 💯