fix some nits
This commit is contained in:
Jeff Regan
GitHub
parent
8433daa9c3
commit
26207ea4ec
|
|
@ -9,8 +9,8 @@ Grooming work for new and existing contributors
|
|||
|
||||
## Background
|
||||
|
||||
A goal of SIG CLI is to keep the SIG open to contributions from anyone willing to do the work to get involved.
|
||||
While many folks have shown interest in contributing, we have struggled to keep most folks engaged.
|
||||
A goal of SIG CLI is to keep the SIG open to contributions from anyone willing to do the work to get involved.
|
||||
While many folks have shown interest in contributing, we have struggled to keep most folks engaged.
|
||||
Kubernetes recently conducted a survey of new contributors, and major themes were that:
|
||||
|
||||
- Contributors don’t know where to start
|
||||
|
|
@ -18,23 +18,19 @@ Kubernetes recently conducted a survey of new contributors, and major themes wer
|
|||
- Communication is hard / everyone is too busy to help
|
||||
- Hard to get reviewers on PRs
|
||||
|
||||
These challenges can be reduced through grooming a work backlog carefully and intentionally. This address the issues by:
|
||||
These challenges can be reduced by:
|
||||
|
||||
- Providing a backlog for contributors to browse and pull work from.
|
||||
- Scoping work that can be done with minimal experience so folks can pick up and work on it. Mark issues as good
|
||||
first time issues if they can be done by someone with no experience.
|
||||
- Scoping work that can be done with minimal experience so folks can pick up and work on it.
|
||||
- Marking issues as good first time issues if they can be done by someone with no experience.
|
||||
- Reducing the need for constant communication by having the work be well defined and clearly scoped in the issues
|
||||
themselves.
|
||||
- Ensure each issue has a stake holder that is committed to seeing it through. This individual will be motivated to
|
||||
make sure it is reviewed.
|
||||
- Ensuring each issue has a stake holder that is committed to seeing that changes are reviewed.
|
||||
|
||||
## Contribution lifecycle
|
||||
|
||||
1. Issue is created with description and labels
|
||||
- Made sure the issue meets guidelines for a good issue
|
||||
- Issue must have a stakeholder invested in the work being completed (stakeholder is either the assignee or if no assignee, the issue creator)
|
||||
2. SIG agrees that work for issue will be accept
|
||||
- Must be low overhead approach
|
||||
1. A [good issue](#what-makes-a-good-issue) is created with description and labels
|
||||
2. SIG agrees that work for issue will be accepted
|
||||
3. Issue moved to [backlog](https://github.com/kubernetes/kubectl/projects/3) in the GH project
|
||||
4. Contributor assigns issue to self, or asks issue be assigned if they are not a Kubernetes org member
|
||||
5. Issue moved to [assigned](https://github.com/kubernetes/kubectl/projects/3) in the GH project
|
||||
|
|
@ -52,17 +48,29 @@ These challenges can be reduced through grooming a work backlog carefully and in
|
|||
|
||||
### What makes a good issue
|
||||
|
||||
#### Stakeholder and Contributor
|
||||
|
||||
A stakeholder typically files the issue, wants to see the work done, and will
|
||||
review PRs that address the issue - or find reviewers.
|
||||
|
||||
The contrbutor is the issue assignee - they provide the PR for review.
|
||||
|
||||
The stakeholder may become the contributor, but then they must find someone else
|
||||
to review the PR. They must find a new stakeholder to review the work and help
|
||||
follow through on issue closure.
|
||||
|
||||
#### Encapsulated
|
||||
|
||||
A good issue in the backlog can be implemented without modifying existing code. This makes it much easier to
|
||||
review and gain consensus on. Issues that require modifying large pieces of existing code are typically hard
|
||||
Issues that require modifying large pieces of existing code are typically hard
|
||||
to accept without multiple reviewers, require a high degree of communication and require knowledge of the
|
||||
existing codebase. This makes them bad candidates for contributors looking to pick up a piece of work on independently.
|
||||
existing codebase.
|
||||
|
||||
This makes them bad candidates for contributors looking to pick up a piece of work on independently.
|
||||
|
||||
Issues with good encapsulation have the following properties:
|
||||
|
||||
- Minimal wiring or changes to existing code
|
||||
- Easy to disable / enable with a flag
|
||||
- Can disable / enable with a flag
|
||||
- Easy to review the contribution on its own without needing to examine other parts of the system
|
||||
- Low chance of needing to rebase or conflicting with changes made in parallel
|
||||
|
||||
|
|
@ -80,7 +88,7 @@ reviewed not the concept. SIG CLI needs to come up with low overhead a process
|
|||
### Code documentation
|
||||
|
||||
Documenting code is an excellent starter task. It is easier to merge and get consensus on than writing tests or
|
||||
features. It also provides a structured approach to learning about the system and its components.
|
||||
features. It also provides a structured approach to learning about the system and its components.
|
||||
For the packages that need it most, understanding the code base well enough to document it may be an involved task.
|
||||
|
||||
- Adding doc.go files to packages that are missing them
|
||||
|
|
@ -91,22 +99,22 @@ For the packages that need it most, understanding the code base well enough to d
|
|||
### Test coverage
|
||||
|
||||
Improving test coverage and augmenting e2e tests with integration tests is also a good candidate for 1st and
|
||||
2nd time contributors. Writing tests for libraries requires understanding how they behave and are meant to be used.
|
||||
2nd time contributors. Writing tests for libraries requires understanding how they behave and are meant to be used.
|
||||
Improving code coverage allows the project to move more quickly by reducing regressions issues that the SIG must field,
|
||||
and by providing a safety net for code reviewers ensuring changes don’t break existing functionality.
|
||||
|
||||
- Write unit tests for functionality only covered by integration and [e2e tests](https://github.com/kubernetes/community/blob/master/contributors/devel/e2e-tests.md)
|
||||
- Integration tests may run processes, such as the apiserver, but do so locally
|
||||
- E2e tests run a full Kubernetes cluster (remote)
|
||||
- Integration tests may run processes, such as the apiserver, but do so locally
|
||||
- E2e tests run a full Kubernetes cluster (remote)
|
||||
- Write integration tests for functionality only covered by [e2e tests](https://github.com/kubernetes/community/blob/master/contributors/devel/e2e-tests.md)
|
||||
- Improve coverage for edge cases and different inputs to functions
|
||||
- Checking behavior for invalid arguments
|
||||
- Refactoring existing tests to pull out common code into reusable functions
|
||||
- **Note:** this should be very well scoped as it impacts existing tests and reviewers need to make sure nothing breaks
|
||||
- _This should be very well scoped as it impacts existing tests and reviewers need to make sure nothing breaks._
|
||||
|
||||
### New libraries
|
||||
|
||||
Encapsulated libraries are great contributions for experienced contributors - either programming in go, or
|
||||
Encapsulated libraries are great contributions for experienced contributors - either programming in Go, or
|
||||
with Kubernetes. Because the libraries are encapsulated, it is easier for reviewers to determine the correctness
|
||||
of their interactions with the existing system. If the functionality is new or can be disabled with a flag, the
|
||||
risk of accepting the change is reduced, and should be easier for reviewers to get consensus on.
|
||||
|
|
@ -127,23 +135,23 @@ on the issue. We use labels to define the most important metadata about the iss
|
|||
|
||||
#### Size
|
||||
- size/S
|
||||
- 4-10 hours
|
||||
> 4-10 hours
|
||||
- size/M
|
||||
- 10-20 hours
|
||||
> 10-20 hours
|
||||
- size/L
|
||||
- 20+ hours
|
||||
> 20+ hours
|
||||
|
||||
#### Type (docs / tests / feature)
|
||||
|
||||
- type/code-cleanup
|
||||
- Usually some refactoring or small rewrites of code.
|
||||
> Usually some refactoring or small rewrites of code.
|
||||
- type/code-documentation
|
||||
- Write doc.go with package overview and examples or document types and functions.
|
||||
> Write doc.go with package overview and examples or document types and functions.
|
||||
- type/code-feature
|
||||
- Usually a new go package / library for some functionality. Should be encapsulated.
|
||||
> Usually a new go package / library for some functionality. Should be encapsulated.
|
||||
- type/code-test-coverage
|
||||
- Audit tests for a package. Run coverage tools and also manually look at what functions are missing unit or
|
||||
integration tests. Write tests for these functions.
|
||||
> Audit tests for a package. Run coverage tools and also manually look at what functions are missing unit or
|
||||
> integration tests. Write tests for these functions.
|
||||
|
||||
### Description
|
||||
|
||||
|
|
@ -156,7 +164,7 @@ on the issue. We use labels to define the most important metadata about the iss
|
|||
1. Contributor messages stakeholder on the issue, and maybe on slack
|
||||
2. Stakeholder moves issue from backlog to assigned
|
||||
3. Contributor updates issue weekly and publishes work in progress to a fork
|
||||
- Fork linked on issue
|
||||
- Fork linked on issue
|
||||
4. Once work is ready for review, contributor files a PR and notifies the stakeholder
|
||||
|
||||
## What is expected as part of contributing a library?
|
||||
|
|
@ -175,4 +183,3 @@ on the issue. We use labels to define the most important metadata about the iss
|
|||
### Ownership of addressing issues
|
||||
|
||||
- Fix bugs discovered in the contribution after it has been accepted
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue