Skip to content
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

Add chart owners #2402

Merged
merged 1 commit into from
Oct 26, 2021
Merged

Conversation

stevehipwell
Copy link
Contributor

@stevehipwell stevehipwell commented Oct 25, 2021

Description

This PR adds an OWNERS file to the ./charts directory to helm make the chart maintenance easier.

This PR will also improves the targeting of chart CI by making the PR CI only run when merging to master, and the release CI will only run when the Chart.yaml has been modified. The CI changes means that it is possible to make chart changes without a release, at the cost of un-tested code making it to master (before a release the code would be tested). This isn't intended behaviour but can help deal with edge cases.

Checklist

  • Unit tests updated
  • End user documentation updated

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 25, 2021
@stevehipwell
Copy link
Contributor Author

/assign @Raffo

@k8s-ci-robot k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 25, 2021
@Raffo
Copy link
Contributor

Raffo commented Oct 25, 2021

Do we really need the ci changes? The ci check on branches allowed us to spot things like the image not being ready, so I feel it has some value. The same applies to the release process, you can always make a PR straight to gh-pages.. Or is there something I don't understand?

@stevehipwell
Copy link
Contributor Author

@Raffo these changes shouldn't make any difference to the way the chart is normally maintained as they're only used if signed off by a maintainer. I'm fine to revert them but I've described them below in more detail.

The release change would allow the README to be updated without releasing the chart if you were happy to merge a failing PR. The lint change would allow PRs to a staging branch to not fail if the chart config hadn't been set correctly, which would allow chart changes to be collected on a temp branch to be merged together into main (with linting on).

Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 26, 2021
@stevehipwell
Copy link
Contributor Author

@Raffo I've removed the CI changes, as even if they're wanted they should be a separate PR with accompanying docs.

@stevehipwell stevehipwell changed the title Add chart owners and improve chart CI Add chart owners Oct 26, 2021
@stevehipwell
Copy link
Contributor Author

@Raffo I've got an open PR on the Metrics Server repo (kubernetes-sigs/metrics-server#877) to update the chart CI that I think would also fit in here. Once this has ben merged I'll open a new PR here with similar changes for discussion if that works for you?

Copy link
Contributor

@Raffo Raffo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Raffo, stevehipwell

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2021
@k8s-ci-robot k8s-ci-robot merged commit 0fa419a into kubernetes-sigs:master Oct 26, 2021
@stevehipwell stevehipwell deleted the chart-owners branch October 26, 2021 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants