Add organization-wide community health files - #19
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (4)
📝 WalkthroughWalkthroughAdds repository contribution, conduct, and security documentation and updates the README to link to the new community health files. ChangesContribution guide and repository policy docs
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
40-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
textlanguage specifier to plain-text fenced code blocks.markdownlint flags these blocks for missing language identifiers. While purely cosmetic, adding
textresolves the warnings and ensures consistent rendering:-``` +```textApplies to the repository tree (line 40), the
Signed-off-byexample (line 167), and the commit message template (line 186).Also applies to: 167-167, 186-186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` at line 40, Update the plain-text fenced code blocks in the CONTRIBUTING.md examples to use the text language specifier instead of an empty fence. Locate the markdown examples for the repository tree, the Signed-off-by example, and the commit message template, and change each affected fenced block to a text-labeled fence so markdownlint no longer reports missing language identifiers.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@CONTRIBUTING.md`:
- Line 40: Update the plain-text fenced code blocks in the CONTRIBUTING.md
examples to use the text language specifier instead of an empty fence. Locate
the markdown examples for the repository tree, the Signed-off-by example, and
the commit message template, and change each affected fenced block to a
text-labeled fence so markdownlint no longer reports missing language
identifiers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd6bf905-2a86-4065-a046-820f1d57bba9
📒 Files selected for processing (2)
CONTRIBUTING.mdREADME.md
| git commit --amend -s | ||
| ``` | ||
|
|
||
| ### Cryptographic Signing (Optional but Encouraged) |
There was a problem hiding this comment.
Addresses conversations and PR from #14 (comment)
|
|
||
| Each repository has an `OWNERS` file listing approvers and reviewers. PRs require both `/lgtm` and `/approve` from two different OWNERS members before they can be merged. | ||
|
|
||
| We use [Prow](https://docs.prow.k8s.io/) to manage CI and merging. You might see maintainers leave comments like: |
There was a problem hiding this comment.
@mpryc Note: Review if this is configured in prow across all repos, so we don't write inaccurate information.
247324a to
a760936
Compare
Add CONTRIBUTING.md, SECURITY.md, and CODE_OF_CONDUCT.md as organization-wide community health files for all medik8s repositories. CONTRIBUTING.md covers development workflow, PR process, code style, testing, commit guidelines (DCO and cryptographic signing), review and approval (Prow commands, two-approval requirement), and security reporting. SECURITY.md enables GitHub's built-in "Report a vulnerability" button. CODE_OF_CONDUCT.md adopts the Contributor Covenant v2.1. Also update README.md to reference the new contributing guide. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Michal Pryc <mpryc@redhat.com>
a760936 to
139fb6f
Compare
mpryc
changed the title
|
|
||
| ``` | ||
| ├── api/ # CRD types, webhooks, deepcopy (v1alpha1 or v1beta1) | ||
| ├── controllers/ or pkg/ # Reconciler logic (location varies by repo) |
There was a problem hiding this comment.
we recently updated to the current kubebuilder structure, which uses cmd/ for main.go and internal/ for all "private" code
There was a problem hiding this comment.
Thank you ! Pulled latest and confirmed. Updated to cmd/ + internal/. I didn't sync with the latest upstream while reviewing it.
|
|
||
| 2. **Make your changes** — keep commits focused and logical. | ||
|
|
||
| 3. **Run the full pre-submit check locally** before pushing: |
There was a problem hiding this comment.
might be worth double checking if this is true for all operators, but make test should be sufficient because it runs all needed checks
There was a problem hiding this comment.
Yeah, I remember some operators have make go-tidy, and some others make tidy and so on
There was a problem hiding this comment.
You are right. In most repos (FAR, NMO, MDR, SBR), make test already runs all needed checks, so you are right to simplify those. I've added instead make help so it's clear what can be used to help identify the targets.
|
|
||
| 1. **Always submit PRs from your personal fork**, not from branches on the main repository. | ||
| 2. **One concern per PR** — don't mix unrelated changes. | ||
| 3. **Fill in the PR description** — explain *what* changed and *why*. |
There was a problem hiding this comment.
we have a template, might be worth mentioning?
| 1. **Always submit PRs from your personal fork**, not from branches on the main repository. | ||
| 2. **One concern per PR** — don't mix unrelated changes. | ||
| 3. **Fill in the PR description** — explain *what* changed and *why*. | ||
| 4. **All CI checks must pass** before merge. The pre-submit pipeline (GitHub Actions) runs: |
There was a problem hiding this comment.
same here, should be make test only
There was a problem hiding this comment.
🧹 Nitpick comments (1)
CONTRIBUTING.md (1)
86-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix broken step numbering.
The workflow steps jump from 3 to 5 — step 4 is missing.
3. **Run tests locally** before pushing. In most repos, `make test` handles formatting, linting, code generation, and unit tests in one step: ```bash make test ``` -5. **Push to your fork** and open a PR against the upstream `main` branch. +4. **Push to your fork** and open a PR against the upstream `main` branch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CONTRIBUTING.md` around lines 86 - 98, The workflow list in CONTRIBUTING.md has broken numbering because the step after the `make test` section skips from 3 to 5. Update the ordered list so the “Push to your fork...” item is numbered as step 4, keeping the sequence consistent in this section.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@CONTRIBUTING.md`:
- Around line 86-98: The workflow list in CONTRIBUTING.md has broken numbering
because the step after the `make test` section skips from 3 to 5. Update the
ordered list so the “Push to your fork...” item is numbered as step 4, keeping
the sequence consistent in this section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a2724497-cddb-4ae1-a9e4-9fc794c95438
📒 Files selected for processing (4)
CODE_OF_CONDUCT.mdCONTRIBUTING.mdREADME.mdSECURITY.md
✅ Files skipped from review due to trivial changes (2)
- SECURITY.md
- CODE_OF_CONDUCT.md
- Update repo structure to current kubebuilder layout (cmd/ + internal/) - Simplify pre-submit workflow: make test covers all checks - Simplify CI steps in PR process to match actual pipelines - Reference PR template in PR description guidance - Add make help to build commands - Expand Prow section: /hold, /cherry-pick, self-approval disabled, GitHub Approve review acts as /lgtm Suggested-by: @slintes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Michal Pryc <mpryc@redhat.com>
15958c7 to
f197915
Compare
|
|
||
| > **Note**: Self-approval is disabled — you cannot `/lgtm` or `/approve` your own PR. | ||
|
|
||
| Reviews can sometimes take a few days. If your PR hasn't received feedback, please don't hesitate to ping the reviewers in the comments! |
There was a problem hiding this comment.
Or ask in the mailing group?
| Reviews can sometimes take a few days. If your PR hasn't received feedback, please don't hesitate to ping the reviewers in the comments! | |
| Reviews can sometimes take a few days. If your PR hasn't received feedback, please don't hesitate to ping the reviewers in the comments or through our Google Group (see at #getting-help)! |
Or a K8s Slack channel that we will create?
There was a problem hiding this comment.
Added Google Group link
There was a problem hiding this comment.
@razo7 having slack on the k8s is a valid point, the guidelines for creating one is:
External projects (ones not owned by a Kubernetes SIG) may have a maximum of two channels, usually #project or #project-users, and #project-dev.
I can request a #medik8s and #medik8s-dev ? Let's discuss this with @weshayutin
- Move "Fixes #123" guidance to PR description (avoids cherry-pick issues) - Change commit body wrap to 72 chars (K8s convention) - Make CoC reporting confidential via OWNERS or private report - Remove Red Hat mention from Security and SECURITY.md - Add /unhold as alternative to /hold cancel - Reword self-approval note to state policy clearly - Add Google Group link to review ping guidance - List PR template and release scripts in README Suggested-by: @razo7 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Michal Pryc <mpryc@redhat.com>
weshayutin
left a comment
weshayutin
left a comment
There was a problem hiding this comment.
@mpryc this is a VERY nice start. Perfection is not required here folks, this work is evolving :)
razo7
left a comment
razo7
left a comment
There was a problem hiding this comment.
this is a VERY nice start. Perfection is not required here folks, this work is evolving :)
I added some small remarks (that we can tackle in a follow-up) but I agree with @weshayutin and TY @mpryc.
|
|
||
| If you discover a security vulnerability in any medik8s project, **do not** open a public issue. | ||
|
|
||
| Please report vulnerabilities confidentially by contacting any of the maintainers listed in the repository's OWNERS file, or use the "Security and quality" tab on the repository to submit a private GitHub Security Advisory. |
There was a problem hiding this comment.
OWNERS file contact path has no private mechanism, as OWNERS files list GitHub usernames only, so no email addresses and GitHub DMs are disabled by default.
There's no viable private contact path through a GitHub username alone, and the medik8s org has no security_email configured. Therefore, as mentioned in the code of conduct, suggest reaching out to a dedicated channel (e.g., medik8s@googlegroups.com with subject "Vulnerability Report").
Consider adding:
- Supported versions — reporters need to know which releases receive security fixes
- Response time — even a rough SLA like "we will acknowledge within 5 business days" sets expectations
|
|
||
| ## Reporting | ||
|
|
||
| If you experience or witness unacceptable behavior, please report it confidentially by contacting any of the maintainers listed in the repository's OWNERS file, or use the "Security and quality" tab on the repository to submit a private report. |
There was a problem hiding this comment.
Conduct violations shouldn't use the security advisory system - maybe the RH team email group or a dedicated channel (e.g., medik8s@googlegroups.com with subject "Code of Conduct Report").
Also: if the person being reported IS an OWNERS member, the reporter has no independent path.
| ├── Makefile # Build targets | ||
| ├── Dockerfile # Multi-stage container build | ||
| ├── OWNERS # Reviewers and approvers | ||
| └── .github/workflows/ # CI pipelines |
There was a problem hiding this comment.
NIT: .github/workflows/ is labeled as "CI pipelines" but medik8s repos primarily use Prow (OpenShift CI) for CI, not GitHub Actions. This could mislead contributors into thinking GitHub Actions is the main CI system.
Suggestion: .github/workflows/ # GitHub Actions (Prow handles primary CI)
|
|
||
| ### Developer Certificate of Origin (DCO) | ||
|
|
||
| We require all commits to be signed off, certifying you have the right to submit the code under the project's Apache 2.0 license. Add a `Signed-off-by` line to your commits: |
There was a problem hiding this comment.
"We require all commits to be signed off" but Medik8s Prow config does not include the dco plugin, and branch protection does not require signed commits. PRs without Signed-off-by will pass CI.
Either:
- Reword to "We encourage all commits to include a Signed-off-by line" (aspirational), or
- Enable the dco Prow plugin first, then document the requirement
I would suggest the second option
There was a problem hiding this comment.
@razo7 it's ok to have follow PR with those addjustments.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abrugaro, eemcmullan, mpryc, razo7, weshayutin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
7 participants
Why we need this PR
Medik8s currently has no organization-wide community health files. This makes it harder for new community contributors to understand our workflow, conventions, and expectations. GitHub's community profile checklist recommends these as standard community health files.
Changes made
CONTRIBUTING.mdcovering: development workflow, PR process (fork-based, two-approval requirement), code style, testing, commit guidelines (DCO and cryptographic signing), review and approval (Prow commands), and security reporting.SECURITY.mdto enable GitHub's built-in "Report a vulnerability" button across all repos.CODE_OF_CONDUCT.mdadopting the Contributor Covenant v2.1.README.mdto reference the new community health files.Which issue(s) this PR fixes
Fixes RHWA-1171
Test plan
Documentation-only change. No code, no tests required.
Summary by CodeRabbit