Thorough overhaul of CONTRIBUTING doc. #24261
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon 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 |
openshift-ci
bot
added
the
approved
|
I'm still poking at things but this should be the core of the changes I wanted to make |
CONTRIBUTING.md
Outdated
| If you find a new issue with the project we'd love to hear about it! | ||
| The most important aspect of a bug report is that it includes enough information for us to reproduce it. | ||
| To make this easier, there are three types of issue templates you can use. | ||
| If you have a bug to report, please use *Bug Report* template. |
There was a problem hiding this comment.
probably bullet or number these and remove the if's
| Note: Older closed issues/PRs are automatically locked. | ||
| If you have a similar problem please open a new issue instead of commenting. | ||
|
|
||
| If you find a new issue with the project we'd love to hear about it! |
There was a problem hiding this comment.
somewhere in here should be noted that a reproducer is the absolute best.
CONTRIBUTING.md
Outdated
| If you have a bug to report, please use *Bug Report* template. | ||
| If you have an idea to propose, please use the *Feature Request* template. | ||
| If your issue is something else, please use the default empty template. | ||
| Please include as much detail as possible and try to remove anything that doesn't really relate to the issue itself. |
There was a problem hiding this comment.
maybe something about not filling out the required fields may hamper our ability to fix things.
CONTRIBUTING.md
Outdated
| If you can not set the label, just add a quick comment in the issue asking that | ||
| the “In Progress” label be set and a member will do so for you. | ||
| Once you have decided to contribute to Podman by working on an issue, check our backlog of [open issues](https:/containers/podman/issues) looking for any that are unassigned. | ||
| If you want to work on a specific issue that is already assigned, but does not appear to be actively being worked on, please ping the assignee in the issue and ask if you can take over. |
There was a problem hiding this comment.
remove the comma before but; also you might consider replacing ping with notify/highlight
| reproduce it, the faster it'll be fixed! | ||
| Before reporting an issue, check our backlog of [open issues](https:/containers/podman/issues) to see if someone else has already reported it. | ||
| If so, feel free to add your scenario, or additional information, to the discussion. | ||
| Or simply "subscribe" to it to be notified when it is updated. |
There was a problem hiding this comment.
pre existing, but maybe add: Please do not add comment saying "+1" or similar without any new information. Instead use the thumbs up reaction on the original report/comment.
Thankfully these comments happen rarely enough so it is not a big problem but maybe nice to mention here.
| If you want to work on a specific issue that is already assigned, but does not appear to be actively being worked on, please ping the assignee in the issue and ask if you can take over. | ||
| If they do not respond after several days, you can ping a maintainer to have the issue reassigned. | ||
| When working on an issue, please assign it to yourself. | ||
| If you lack permissions to do so, you can ping the `@containers/podman-maintainers` group to have a maintainer set you as assignee. |
There was a problem hiding this comment.
The openshift bit should be able to assign via a /assign comment which does not need permission from the user AFAIK, maybe test that and document that so we do not have to deal with it.
| This section describes how to make a contribution to Podman. | ||
| These instructions are geared towards using a Linux development machine, which is required for doing development on the Podman backend. | ||
| Development for the Windows and Mac clients can also be done on those operating systems. | ||
| Check out these instructions for building the Podman client on [MacOSX](./build_osx.md) or [Windows](./build_windows.md). | ||
|
|
||
| ### Prepare your environment | ||
|
|
||
| Read the [install documentation to see how to install dependencies](https://podman.io/getting-started/installation#build-and-run-dependencies). |
There was a problem hiding this comment.
unrelated by they must be updated over there at some point too.
CONTRIBUTING.md
Outdated
|
|
||
| ### Fork and clone Podman | ||
|
|
||
| First you need to fork this project on GitHub. | ||
| First, you need to fork this project on GitHub. | ||
|
|
||
| Be sure to have [defined your `$GOPATH` environment variable](https:/golang/go/wiki/GOPATH). |
There was a problem hiding this comment.
This paragraph here and below is outdated, with modern go modules this stuff is really not needed
CONTRIBUTING.md
Outdated
| This code is included in the Podman repository, but is actually maintained elsewhere. | ||
| Pull requests that change the vendor/ directory directly will not be accepted. | ||
| Instead, changes should be submitted to the original package (defined by the path in `vendor/`; for example, `vendor/github.com/containers/storage` is the [containers/storage library](https:/containers/storage/). | ||
| Once the changes have been merged into the original package, Podman's vendor directory can be updated by using `go get` on the appropriate version of the package, then running `make vendor-in-container`. |
There was a problem hiding this comment.
make vendor or make vendor-in-container, I really do not see why a container is needed for that
| Podman provides an extensive suite of regression tests in the `test/` directory. | ||
| There is a [readme](https:/containers/podman/blob/main/test/README.md) file available with details about the tests and how to run them. | ||
| All pull requests should be accompanied by test changes covering the changes in the PR. | ||
| Pull requests without tests cannot be merged without maintainer intervention. |
There was a problem hiding this comment.
Well that is a bit of a pointless sentence as a maintainer must review every change anyway
CONTRIBUTING.md
Outdated
| ### Integration Tests | ||
|
|
||
| Our primary means of performing integration testing for Podman is with the | ||
| [Ginkgo](https:/onsi/ginkgo) BDD testing framework. This allows | ||
| us to use native Golang to perform our tests and there is a strong affiliation | ||
| between Ginkgo and the Go test framework. Adequate test cases are expected to | ||
| be provided with PRs. | ||
|
|
||
| For details on how to run the tests for Podman in your test environment, see the | ||
| testing [README.md](test/README.md). | ||
|
|
||
| ### System Tests | ||
|
|
||
| The system tests are written in Bash using the BATS framework. | ||
| They provide less comprehensive coverage than the integration tests. | ||
| They are intended to validate Podman builds before they are shipped by distributions. |
There was a problem hiding this comment.
IF you document this here you properly want to show the directory path were they are defined.
The doc has been reorganized and reordered. New sections have been added as necessary to cover things not covered by the old guide. Some sections were expanded (e.g. detailing differences between E2E and System tests). Some sections that we did not actually follow were removed. Fixes https://issues.redhat.com/browse/RUN-2281 Signed-off-by: Matt Heon <[email protected]>
mheon
force-pushed
the
update_contributing
branch
from
2725429
to
187f0c9
Compare
The doc has been reorganized and reordered. New sections have been added as necessary to cover things not covered by the old guide. Some sections were expanded (e.g. detailing differences between E2E and System tests). Some sections that we did not actually follow were removed.
Fixes https://issues.redhat.com/browse/RUN-2281
Does this PR introduce a user-facing change?