Consolidate Metal3 Contributing guide #40
Conversation
This completes the consolidation effort where generic contributing guidelines are maintained in the community repo and individual repos only contain repo-specific information. Signed-off-by: Kashif Khan <kashif.khan@est.tech>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
metal3-io-bot
added
the
size/L
|
/hold For discussion and proper review |
metal3-io-bot
added
the
do-not-merge/hold
|
/cc @tuminoid |
tuminoid
left a comment
tuminoid
left a comment
There was a problem hiding this comment.
Plenty of small nits, nothing major.
I also commented on each repo based ones. They were varying wildly, and IMO they should be pretty close to each other.
Mentioning AGENTS.md here would be nice addition too.
| - [baremetal-operator](https://github.com/metal3-io/baremetal-operator/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) | ||
| - [cluster-api-provider-metal3](https://github.com/metal3-io/cluster-api-provider-metal3/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) | ||
| - [ip-address-manager](https://github.com/metal3-io/ip-address-manager/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) | ||
| - [ironic-standalone-operator](https://github.com/metal3-io/ironic-standalone-operator/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22) |
There was a problem hiding this comment.
We have many more repos than just the four, we should have single link that list all /help-wanted and /good-first-issues and maybe third that shows all issues.
There was a problem hiding this comment.
Is it possible to search through issues across the org? If not, I think we need to add a project that automatically picks up issues with the relevant labels. That should work well across the org. For now we just have the roadmap: Metal3 - Roadmap (view)
|
|
||
| ## Contributing a Patch | ||
|
|
||
| 1. If you haven't already done so, sign a Contributor License Agreement |
| Individual commits should not be tagged separately, but will generally be | ||
| assumed to match the PR. For instance, if you have a bugfix along with a | ||
| breaking change, it's generally encouraged to submit the bugfix separately, | ||
| but if you must put them in one PR, mark the commits separately. |
There was a problem hiding this comment.
emojis don't go with commits, they go with PR only.
| ## Pull Request Labels | ||
|
|
||
| All code PRs should be labeled with one of the following to indicate the type of | ||
| change: |
There was a problem hiding this comment.
We could elaborate that the emoji should match the "product" change. Bug fixes to CI are still just seedlings to avoid polluting the changelog etc.
| docs](https://github.com/kubernetes/community/tree/master/contributors/devel). | ||
|
|
||
| Expect reviewers to request that you avoid common [Go style | ||
| mistakes](https://github.com/golang/go/wiki/CodeReviewComments) in your PRs. |
There was a problem hiding this comment.
We should elaborate the code review process here more. /ok-to-test, workflows, /lgtm and /approve flow etc.
There was a problem hiding this comment.
Would be good to also mention that commits should generally be squashed and that all tests are expected to be passing. Perhaps also worth mentioning that there are usually linters, formatters, etc that can be easily run locally before push.
|
|
||
| Common commands include: | ||
|
|
||
| - `/lgtm` - Adds the `lgtm` label (Looks Good To Me) |
| Common commands include: | ||
|
|
||
| - `/lgtm` - Adds the `lgtm` label (Looks Good To Me) | ||
| - `/approve` - Adds the `approve` label |
| - `/retest` - Reruns failed tests | ||
| - `/assign @username` - Assigns an issue or PR | ||
| - `/cc @username` - Requests a review | ||
|
|
There was a problem hiding this comment.
add /test xxx, with mention of /test ? to get a list
|
|
||
| ## Release Process | ||
|
|
||
| ### General Release Guidelines |
There was a problem hiding this comment.
Main releasing.doc is unliked, while we do link the four repos individual release docs.
| - [ironic-standalone-operator releasing docs](https://github.com/metal3-io/ironic-standalone-operator/blob/main/docs/releasing.md) | ||
| - Check individual project repos for their specific release documentation | ||
|
|
||
| ## Google Doc Viewing Permissions |
| - **Minor and patch releases** are generally done immediately after a feature or | ||
| bugfix is landed, or sometimes a series of features tied together. |
There was a problem hiding this comment.
Isn't it more like monthly? We don't push releases as soon as there is anything merged to a release branch
4 participants
This completes the consolidation effort where generic contributing guidelines are maintained in the community repo and individual repos only contain repo-specific information.
Fixes #28