Create initial documentation #2
Conversation
gmfrasca
force-pushed
the
init-docs
branch
3 times, most recently
from
599b94b to
e3824cd
Compare
hbelmiro
left a comment
hbelmiro
left a comment
There was a problem hiding this comment.
I left some comments.
Besides that, I think it will be easier for both contributors and maintainers if we keep the content of TESTING.md and ONBOARDING.md all in CONTRIBUTING.md.
We already have some duplicates between them.
gmfrasca
changed the title
|
/lgtm |
Signed-off-by: Giulio Frasca <[email protected]>
Signed-off-by: Giulio Frasca <[email protected]>
Signed-off-by: Giulio Frasca <[email protected]>
Signed-off-by: Giulio Frasca <[email protected]>
Signed-off-by: Giulio Frasca <[email protected]>
Signed-off-by: Giulio Frasca <[email protected]>
|
/retest (I believe there was formerly a fixup commit that got eventually did get autosquashed in, but now the tide check is stuck on failed status because it thinks this is a squash-and-merge PR) |
|
/retest |
docs/GOVERNANCE.md
Outdated
|
|
||
| ### Core Tier | ||
|
|
||
| **Officially supported components** maintained by the Kubeflow community. |
There was a problem hiding this comment.
The word "community" is becoming a big ambiguous, because you say Core Tier is supposted by Kubeflow Community, whereas Third Party components are Community-contributed components with lighter requirements. I think people will start getting confused.
I propose we use the following explicit terminology:
Core Tier: Kubeflow Core Maintainers. (define what this means)
Third Party Tier: Kubeflow Third Party Maintainers (define what this means)
You should also mention that the Root governance of the Repository - for example the APPROVERS in the owners file at the root repository lies with primarily KFP maintainers. This is not the same as Core maitnainers. We can address whether we want to expand this governance later.
Think about a new team wanting to use the repo and control /own their component, how can we give them a quick run down?
docs/ONBOARDING.md
Outdated
|
|
||
| ### Quick Reference | ||
|
|
||
| ```bash |
There was a problem hiding this comment.
Refer back to the KEP. These are not aligned with the linting tools/requirements in the KEP.
docs/GOVERNANCE.md
Outdated
|
|
||
| *Key roles and responsibilities for governing and maintaining the repository.* | ||
|
|
||
| ### KFP Component Core Maintainers |
There was a problem hiding this comment.
So we have:
- KFP Component Core Maintainers
- KFP Component Third-Party Maintainers
- Component Owners
This is very confusing. I think just do this:
- Repo maintainers (alternatively you can say Project Maintainers)
- Required approver in the root OWNERS file
- Responsible for orchestrating releases
- Setting roadmaps and accepting KEPs
- Responsible for managing the overall project, issues, and other repository maintenance duties.
- Core Component maintainers
- Require approver for at least one core component OWNERS file
- (the rest of what you have written for
Component Ownerssection
- Third Party Component maintainers
- (same as core component maintainers but for third party components)
you can reword/update the text as needed, but just note that repo maintainers don't have to be responsible for components (these are the individuals that have admin access on the repo)
docs/GOVERNANCE.md
Outdated
| ### KFP Component Core Maintainers | ||
|
|
||
| Core Maintainers are responsible for the stewardship of Core-tier components. Their key responsibilities include: | ||
| - Defined by having an entry in the `approvers` section of the `OWNERS` file in either the repo root, `components/OWNERS`, and/or `pipelines/OWNERS` file(s) |
There was a problem hiding this comment.
Note third core/third-party maintainers don't have a requirement for having approvers in OWNERS, that's just for repo maintainers.
It also doesn't mean Repo Maintainers are automatically component owners, one person can be both (if they are approvers in multiple owner files), but they are not always both - we don't have to explicitly mention this.
There was a problem hiding this comment.
correct me if i'm wrong, but does OWNERship not cascade down? so if you are listed as an approver in the OWNERS of repo root (ie a Repo Maintainer), you also have approver permissions for all the directories in that repo, including components and pipelines?
There was a problem hiding this comment.
It does but that just means repo owners should be careful not to act like component owners, even if they have the permissions to do so. As an analogy consider Kubeflow Org admins, they have admin rights to all KF repos, but they are not automatically considered maintainers of KF components. It's similar.
docs/GOVERNANCE.md
Outdated
| 1. **Nomination**: Any contributor can nominate via GitHub issue | ||
| 3. **Requirements**: Validate component meets all core tier requirements | ||
| 4. **Decision**: Community and core-component Maintainer consensus approval | ||
| 5. **Timeline**: 4-6 weeks review process |
There was a problem hiding this comment.
this timeline looks arbitrary, where did we get it from? can we remove it?
There was a problem hiding this comment.
Doesn't appear that any promotion/demotion mechanic was specified by the KEP, so perhaps lets just totally remove these sections?
There was a problem hiding this comment.
I'm fine with removing it to keep the doc minimal
docs/GOVERNANCE.md
Outdated
| - **Policy**: KFP Component Core Maintainers | ||
| - **Strategic**: KFP Component Core Maintainers |
There was a problem hiding this comment.
Update to repo maintainers once you make the above amendments
docs/GOVERNANCE.md
Outdated
|
|
||
| ### Process | ||
| 1. **Proposal**: Create GitHub issue/RFC | ||
| 2. **Discussion**: Community feedback (1-2 weeks) |
There was a problem hiding this comment.
| 2. **Discussion**: Community feedback (1-2 weeks) | |
| 2. **Discussion**: Community feedback |
docs/CONTRIBUTING.md
Outdated
| ## Naming Conventions | ||
|
|
||
| - **Components and pipelines** use `snake_case` (e.g., `data_preprocessing`, `model_trainer`) | ||
| - **Git branches** use `component/name` for features or `fix/issue-123` for bug fixes |
There was a problem hiding this comment.
is this for user branches during PRs? we don't need to enforce a rule here
|
|
||
| Your `metadata.yaml` must include these fields: | ||
|
|
||
| ```yaml |
There was a problem hiding this comment.
@alyssacgoins can you verify this adheres to the validation script requirements
There was a problem hiding this comment.
@HumairAK @gmfrasca a couple things to be updated:
- name should come first, before tier
- ci.skip_dependency_probe is optional
- ci.compile_check should be removed
- ci.pytest should be removed
- packages_to_install should be removed
- links is optional, and the headings can be custom
Here's the updated metadata.yaml file:
name: my_component
tier: core # or 'third_party'
stability: stable # 'alpha', 'beta', or 'stable'
dependencies:
kubeflow:
- name: Pipelines
version: '>=2.5'
external_services: # Optional list of external dependencies
- name: Argo Workflows
version: "3.6"
tags: # Optional keywords for discoverability
- training
- evaluation
lastVerified: 2025-11-18T00:00:00Z # Updated annually; components are removed after 12 months without update
ci:
skip_dependency_probe: false # Optional. Set true only with justification
links: # Optional, can use custom key-value (not limited to documentation, issue_tracker)
documentation: https://kubeflow.org/components/my_component
issue_tracker: https://github.com/kubeflow/pipelines-components/issues
There was a problem hiding this comment.
hey @alyssacgoins, regarding above
- item order in yaml dicts are not guaranteed - I can update the example here for clarity but the validation script should not assume
nameis first. ci.compile_check,ci.pytestare spec'd as an optional fields in the KEP, are we saying that's an update being made to the KEP spec as well?- I can add
# Optionalcomments to the optional fields such aslinks
| ### OWNERS File | ||
| The OWNERS file enables component owners to self-service maintenance tasks including approvals, metadata updates, and lifecycle management: |
There was a problem hiding this comment.
can you either link to the k8s docs or add a brief description on /lgtm and /approve semantics
as I believe we intend to use prow like the rest of the kf projects
docs/CONTRIBUTING.md
Outdated
| ### Pre-Submission Checklist | ||
|
|
||
| Verify your contribution meets all requirements before requesting review: | ||
|
|
||
| - [ ] All tests pass | ||
| - [ ] Pre-commit hooks pass without errors | ||
| - [ ] All required files are present and complete | ||
| - [ ] `metadata.yaml` includes fresh `lastVerified` timestamp | ||
| - [ ] OWNERS file lists appropriate maintainers | ||
| - [ ] README provides clear documentation with usage examples | ||
| - [ ] Component follows `snake_case` naming convention | ||
| - [ ] (Core tier only) Containerfile present if using custom image | ||
| - [ ] No security vulnerabilities in dependencies | ||
| - [ ] Container builds successfully (if applicable) |
There was a problem hiding this comment.
this should be a checklist in the PR template it self and not added here, otherwise we have to update two places everytime the pr template is updated
There was a problem hiding this comment.
ok, i'll remove this here, and put this in .github/pull_request_template.md, if it's not already being created. Either way, this would be a seperate PR
There was a problem hiding this comment.
okay, just ensure it's captured in some Jira or somewhere so we don't forget
Signed-off-by: Giulio Frasca <[email protected]>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: HumairAK 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 |
5 participants
No description provided.