Thorough overhaul of CONTRIBUTING doc. #24261

mheon · 2024-10-14T14:41:53Z

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?

NONE

mheon · 2024-10-14T14:42:12Z

I'm still poking at things but this should be the core of the changes I wanted to make

baude · 2024-10-14T15:42:25Z

CONTRIBUTING.md

+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.


probably bullet or number these and remove the if's

CONTRIBUTING.md

baude · 2024-10-14T15:45:12Z

CONTRIBUTING.md

+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.


maybe something about not filling out the required fields may hamper our ability to fix things.

baude · 2024-10-14T15:46:46Z

CONTRIBUTING.md

-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://github.com/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.


remove the comma before but; also you might consider replacing ping with notify/highlight

CONTRIBUTING.md

Luap99 · 2024-10-14T15:45:01Z

CONTRIBUTING.md

+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).


unrelated by they must be updated over there at some point too.

Luap99 · 2024-10-14T15:46:36Z

CONTRIBUTING.md


 ### 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://github.com/golang/go/wiki/GOPATH).


This paragraph here and below is outdated, with modern go modules this stuff is really not needed

Luap99 · 2024-10-14T15:48:16Z

CONTRIBUTING.md

+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://github.com/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`.


make vendor or make vendor-in-container, I really do not see why a container is needed for that

Luap99 · 2024-10-14T15:49:18Z

CONTRIBUTING.md

+Podman provides an extensive suite of regression tests in the `test/` directory.
+There is a [readme](https://github.com/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.


Well that is a bit of a pointless sentence as a maintainer must review every change anyway

Maybe "Pull requests without tests require additional scrutiny and waiving."?

Luap99 · 2024-10-14T15:50:58Z

CONTRIBUTING.md

+### Integration Tests
+
+Our primary means of performing integration testing for Podman is with the
+[Ginkgo](https://github.com/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.


IF you document this here you properly want to show the directory path were they are defined.

Luap99

LGTM, nice work

edsantiago

LGTM. Very minor nits.

edsantiago · 2024-10-15T12:16:10Z

CONTRIBUTING.md

+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.
+* 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.


The actual string that appears in the New issue menu is Blank issue, not empty.

edsantiago · 2024-10-15T12:19:18Z

CONTRIBUTING.md


 Please don't include any private/sensitive information in your issue!
+Security issues should NOT be reported via Github and should instead be reported via the process described in our [README](https://github.com/containers/podman/blob/main/README.md#communications).


The New issue > Report a security vulnerability menu entry links instead to SECURITY.md. Should we be consistent?

edsantiago · 2024-10-15T12:26:23Z

CONTRIBUTING.md

+Podman provides an extensive suite of regression tests in the `test/` directory.
+There is a [readme](https://github.com/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.


Maybe "Pull requests without tests require additional scrutiny and waiving."?

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 <mheon@redhat.com>

Luap99

/lgtm

openshift-ci · 2024-10-17T10:59:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, Luap99, 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:

~~OWNERS~~ [Luap99,edsantiago,mheon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Change the DCO wording so that it is clear that we do not need the real name, just someting the properly identifies that person within the community. Touch up the old Reporting Issues and Submitting Pull Requests section based on Matt's work in podman[1] and add some extra step in there. Add new sections for the vendor process and and how to use git bisect. [1] containers/podman#24261 Signed-off-by: Paul Holzinger <pholzing@redhat.com>

openshift-ci bot added release-note-none approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 14, 2024

baude reviewed Oct 14, 2024

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

baude reviewed Oct 14, 2024

View reviewed changes

Luap99 reviewed Oct 14, 2024

View reviewed changes

mheon force-pushed the update_contributing branch from 2725429 to 187f0c9 Compare October 14, 2024 17:11

Luap99 approved these changes Oct 15, 2024

View reviewed changes

edsantiago approved these changes Oct 15, 2024

View reviewed changes

mheon force-pushed the update_contributing branch from 187f0c9 to 1bf37ee Compare October 15, 2024 15:23

Luap99 approved these changes Oct 17, 2024

View reviewed changes

openshift-ci bot assigned Luap99 Oct 17, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 17, 2024

openshift-merge-bot bot merged commit 993ecd5 into containers:main Oct 17, 2024
35 checks passed

Luap99 mentioned this pull request Oct 18, 2024

update CONTRIBUTING.md containers/common#2207

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Thorough overhaul of CONTRIBUTING doc. #24261

Thorough overhaul of CONTRIBUTING doc. #24261

mheon commented Oct 14, 2024

mheon commented Oct 14, 2024

baude Oct 14, 2024

baude Oct 14, 2024

baude Oct 14, 2024

Luap99 Oct 14, 2024

Luap99 Oct 14, 2024

Luap99 Oct 14, 2024

Luap99 Oct 14, 2024

edsantiago Oct 15, 2024

Luap99 Oct 14, 2024

Luap99 left a comment

edsantiago left a comment

edsantiago Oct 15, 2024

edsantiago Oct 15, 2024

edsantiago Oct 15, 2024

Luap99 left a comment

openshift-ci bot commented Oct 17, 2024


		Please don't include any private/sensitive information in your issue!
		Security issues should NOT be reported via Github and should instead be reported via the process described in our [README](https://github.com/containers/podman/blob/main/README.md#communications).

Thorough overhaul of CONTRIBUTING doc. #24261

Thorough overhaul of CONTRIBUTING doc. #24261

Conversation

mheon commented Oct 14, 2024

Does this PR introduce a user-facing change?

mheon commented Oct 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Luap99 left a comment

Choose a reason for hiding this comment

edsantiago left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Luap99 left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Oct 17, 2024