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

Updating the CONTRIBUTING.md to match what we have in Helm, I also ad… #136

Merged
merged 5 commits into from
Jul 26, 2024

Conversation

robertsirc
Copy link
Contributor

Updating the CONTRIBUTING.md to match what we have in Helm and added a section to denote the requirements for GPG.

…ded the requirements for GPG.

Signed-off-by: Robert Sirchia <[email protected]>
Copy link
Contributor

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. This plugin is MUCH simpler in terms of workflow than Helm itself. The sign-off, signing, and size labels are good. Most of the rest of the workflow stuff can be removed as it doesn't happen on this plugin.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@robertsirc
Copy link
Contributor Author

@mattfarina all your changes have been made.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Signed-off-by: Robert Sirchia <[email protected]>
Copy link
Member

@scottrigby scottrigby left a comment

Choose a reason for hiding this comment

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

@robertsirc thanks for this!

It somehow feels wrong to duplicate so much from https://github.com/helm/helm/blob/main/CONTRIBUTING.md 🤔 But given that sections of helm core don't apply here, I can see why this was done.

Hear me out… what if this doc starts off by saying something like

While this helm-mapkubeapis plugin draws its contributing workflow steps from Helm core contributing, this plugin's workflow is much simpler. So we will link only to the sections from Helm core contributing doc that apply to this plugin. There are several exceptions, where we need to make changes from the corresponding steps in Helm core:

  • "Reporting a Security Issue" - link differs (the correct issue queue for this plugin)
  • "How to Contribute a Patch" - this plugin has a limited scope, and does not need to use the HIP process
  • "Pull Requests" - Helm core has a necessarily complex PR lifecycle, while this Plugin uses a standard maintainer PR review process
  • "Labels" - this plugin has different labels for a simpler workflow

Then - keeping your headers below - replace all other section content with a link to the applicable sections in Helm core, like:

## Sign-off On Your Work

See https://github.com/helm/helm/blob/main/CONTRIBUTING.md#sign-your-work

## Support Channels

See https://github.com/helm/helm/blob/main/CONTRIBUTING.md#support-channels

## Semantic Versioning

See https://github.com/helm/helm/blob/main/CONTRIBUTING.md#semantic-versioning

## Issues

See https://github.com/helm/helm/blob/main/CONTRIBUTING.md#issues

etc…

WDYT?

PS, either way, I think we should add the "Signing Your Work" (GPG signing) section – and clarifying header change "Sign Your Work" -> "Sign-off On Your Work" – upstream to Helm core CONTRIBUTING.md.

@robertsirc
Copy link
Contributor Author

I am ok with the approach. @mattfarina thoughts?

CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Contributor

@mattfarina mattfarina left a comment

Choose a reason for hiding this comment

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

@scottrigby I think this is ok to merge as is. It's different enough from helm/helm that pointing to helm/helm will just confuse people. It has not followed the same conventions as helm/helm.

@mattfarina mattfarina merged commit a9ecb2f into helm:main Jul 26, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants