-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restructure repo to host chart on gh-pages #15
Restructure repo to host chart on gh-pages #15
Conversation
Signed-off-by: Brad McCoy <bradmccoydev@gmail.com>
- uses: actions/checkout@8e5e7e5ab8b370d6c329ec480221332ada57f0ab # v3 | ||
- uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I ask why are you using commit hash for the action instead of the version, like:
- uses: actions/checkout@v3
- uses: actions/setup-go@v4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a security best practice for open source projects that people use you can google the reason
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
an example blog here: https://michaelheap.com/ensure-github-actions-pinned-sha/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this article is not a best practice. It's just some points of view. None of the reference link in this article from github action official. Hence please rollback those lines. It just try to sales some 3rd party github action or personal point.
@@ -1,8 +1,8 @@ | |||
apiVersion: v2 | |||
name: kepler | |||
name: kepler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bradmccoydev relocating the Chart.yaml
causes the integration test failure, can you fix that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure I have added the default working directory for the action and it will now run in the new folder structure.
I have also improved the action to run a test again if someone pushes back to the PR to ensure that their change still works:
on:
pull_request:
types:
- "opened"
- "edited"
- "synchronize"
I tested this on my test repo also.
@bradmccoydev thanks! Once the CI is green, it is a a merge click away :D |
Signed-off-by: Brad McCoy <bradmccoydev@gmail.com>
@bradmccoydev thanks, the 0.3.1 chart is in the gh release and gh pages now. |
great news thanks to you @rootfs and @yellowhat for your help. I will make another issue tomorrow for the artifacthub for discussion it will bring more adoption for the project linking it to this chart |
This PR closes #7
I have made a test repo to demonstrate how it works. It can be found here: https://github.com/bradmccoydev/kepler-test/tree/main
Changes made to the directory structure are necessary for the helm releaser to work. This is how we use it on our other CNCF projects that I am a part of.
Details of how to install the chart can be found in the Readme.MD
Next i can make an issue to host it on artifact and that will be good as it will lead to more adoption.
Any Questions please let me know. cc @rootfs @yellowhat