[V6] Add contributing guides and improve documentation #1711

AlbaHerrerias · 2023-12-13T13:02:16Z

This PR includes several contribution guides, implements a code of conduct and adds some improvements in documentation.

Thank you

Co-Authored-By: Alex Feyerke <391124+espy@users.noreply.github.com> Co-Authored-By: Ayham Kteash <11707377+ayhamthemayhem@users.noreply.github.com>

Co-Authored-By: Alex Feyerke <391124+espy@users.noreply.github.com>

twiss

Hey 👋 Happy new year, and thanks for the added docs! Some comments below :)

twiss · 2024-01-04T15:29:53Z

.github/ISSUE_TEMPLATE/BUG_REPORT.md

+
+Before creating a new bug report, please take a few moments to review the following:
+- Have you searched existing issues and discussions about your problem? Your problem might be already solved, and this could avoid creating duplicates
+- Is this a security related bug? If so, disclose it privately following the procedure described in https://github.com/openpgpjs/openpgpjs/blob/main/SECURITY.md


Very nitpicky but in OpenPGP.js, any bug can be considered related to security, but only vulnerabilities (or bugs that could cause them) should be reported there.

Suggested change

- Is this a security related bug? If so, disclose it privately following the procedure described in https://github.com/openpgpjs/openpgpjs/blob/main/SECURITY.md

- Could this issue cause a security vulnerability? If so, please disclose it privately following the procedure described in https://github.com/openpgpjs/openpgpjs/blob/main/SECURITY.md

twiss · 2024-01-04T15:31:38Z

.github/PULL_REQUEST_TEMPLATE.md

+-->
+
+**Description:**
+<!-- Explain what this PR does and why we need it  -->


Suggested change

twiss · 2024-01-04T15:34:23Z

CONTRIBUTING.md

+   ```bash
+   # Clone your fork of the repo into the current directory
+   git clone https://github.com/<your-username>/<repo-name>
+
+   # Navigate to the newly cloned directory
+   cd <repo-name>
+
+   # Assign the original repo to a remote called "upstream"
+   git remote add upstream https://github.com/openpgpjs/<repo-name>
+   ```


Are these instructions meant to be generic to other repos? Since we're in the openpgpjs repo here I would just write that:

Suggested change

```bash

# Clone your fork of the repo into the current directory

git clone https://github.com/<your-username>/<repo-name>

# Navigate to the newly cloned directory

cd <repo-name>

# Assign the original repo to a remote called "upstream"

git remote add upstream https://github.com/openpgpjs/<repo-name>

```

```bash

# Clone your fork of the repo into the current directory

git clone https://github.com/<your-username>/openpgpjs

# Navigate to the newly cloned directory

cd <repo-name>

# Assign the original repo to a remote called "upstream"

git remote add upstream https://github.com/openpgpjs/openpgpjs

```

twiss · 2024-01-04T15:36:36Z

CONTRIBUTING.md

+
+6. Make sure to update or add to the tests when appropriate. Run the appropriate testing suites to check that all tests pass after you've made changes. You can read about our different types of tests in our [Testing](#testing) section.
+
+7. If you added or changed a feature, make sure to document it accordingly in the [README.md](https://github.com/openpgpjs/openpgpjs/blob/main/README.md) file.


Suggested change

7. If you added or changed a feature, make sure to document it accordingly in the [README.md](https://github.com/openpgpjs/openpgpjs/blob/main/README.md) file.

7. If you added or changed a feature, make sure to document it accordingly in the [README.md](https://github.com/openpgpjs/openpgpjs/blob/main/README.md) file, when appropriate.

(not every feature needs to be documented in the main README file)

twiss · 2024-01-04T15:43:02Z

CONTRIBUTING.md

+We follow the seven rules of a great Git commit message, which is extensively described in [this blog post](https://cbea.ms/git-commit/), but in short the principles are:
+
+1. Separate subject from body with a blank line
+2. Limit the subject line to 50 characters
+3. Capitalize the subject line
+4. Do not end the subject line with a period
+5. Use the imperative mood in the subject line
+6. Wrap the body at 72 characters
+7. Use the body to explain what and why vs. how
+
+You can also take a look at our [main branch commits](https://github.com/openpgpjs/openpgpjs/commits/main).


We don't religiously follow all of these, particularly the subject line length limit. So I would just say:

Suggested change

We follow the seven rules of a great Git commit message, which is extensively described in [this blog post](https://cbea.ms/git-commit/), but in short the principles are:

1. Separate subject from body with a blank line

2. Limit the subject line to 50 characters

3. Capitalize the subject line

4. Do not end the subject line with a period

5. Use the imperative mood in the subject line

6. Wrap the body at 72 characters

7. Use the body to explain what and why vs. how

You can also take a look at our [main branch commits](https://github.com/openpgpjs/openpgpjs/commits/main).

We roughly follow the Git commit guidelines in [this blog post](https://cbea.ms/git-commit/).

Additionally, please try to follow the style of the commit messages on our [main branch](https://github.com/openpgpjs/openpgpjs/commits/main).

twiss · 2024-01-04T16:41:58Z

MAINTAINERS.md

+
+### Step 3: Assign the Work
+
+It should be clear who is responsible for working on an issue once it has been triaged and deemed actionable. This also includes eventual reviews. Ideally don’t assign things to people who aren’t expecting it.


The above also means that this isn't always possible. If one of the maintainers will do it, then yes, but otherwise it's up to the applications and contributors to pick up the work, not for us to assign it.

twiss · 2024-01-04T16:43:12Z

MAINTAINERS.md

+
+### Step 4: Follow up
+
+- **If no PR is opened on an issue within a month**, a maintainer should contact the assignee and ask them to create a PR or unassign themselves


This also seems overly rigid. Obviously, if someone else comes along and wants to work on the issue we can re-assign, but otherwise unassigning doesn't seem strictly necessary?

twiss · 2024-01-04T16:46:05Z

MAINTAINERS.md

+Triage can happen asynchronously and continuously, or in regularly scheduled meetings.
+
+**Benefits of triaging are:**
+- Maintaining project momentum
+- Engagement of contributors by increasing responsiveness
+- Prevents piling up of issues and work
+- Prevents issues and the project itself becoming stale
+- Provides structure and an opportunity for collaborative decision-making with regards to the project trajectory and roadmap
+
+It can be beneficial to have triage be part of already existing regular meetings, and if the volume of incoming issues requires it, have a regular, public triage meeting. If there is a regular meeting, make sure it is added here and in the [CONTRIBUTING.md](/CONTRIBUTING.md). Also consider adding it to the [NEW-CONTRIBUTORS.md](/NEW-CONTRIBUTORS.MD) and explicitly invite new and prospective contributors to listen in or participate. This will give interested people a plannable, social opportunity to meet the maintainers and gain insight into the project, with both a low barrier to entry for the new contributors and a low amount of additional effort for the maintainers.


This would seem like a reasonable suggestion if we had ore maintainers, but at the moment Lara is doing most of the work, so then having a meeting seems overkill (although we do have meetings, but not specifically for this).

Also, even if it is/becomes a good suggestion, it would make more sense to me for this document to describe the current practice? So I would drop this for now.

it would make more sense to me for this document to describe the current practice? So I would drop this for now.

IMO it could make sense to have this document as guideline for the future -- or more in general as an "action plan".
Personally I found these suggestions to be quite insightful, and I can definitely see how they would benefit in terms of engagement. Indeed, currently we don't have the volume of issues, nor the triaging need, to justify such meetings (but perhaps we did in the past, when it came to the Node.js streaming issues, for instance).
Still, the way I read it, the language in the document does cover the current practice as well ("Triage can happen asynchronously and continuously"), while also offering a [sound] alternative if need be , without implying any hard requirements 🙂

From the current text it's not really clear which option is current practice and which is a suggestion, though. And I think for OpenPGP.js contributors and future maintainers it would be more useful to know the actual process being followed here, rather than an aspirational document - we can have that elsewhere :) Or just refer to https://github.com/kubernetes/community/blob/master/contributors/guide/issue-triage.md (which is already linked) for future ideas in case the project grows significantly.

twiss · 2024-01-04T16:46:36Z

MAINTAINERS.md

+### Step 4: Follow up
+
+- **If no PR is opened on an issue within a month**, a maintainer should contact the assignee and ask them to create a PR or unassign themselves
+- **If a PR is ready for review**, use your triage meeting to find someone to review it within a reasonable amount of time. If you cannot manage a review soon, explain that to the contributor so they’re not left hanging and know what to expect.


Suggested change

- **If a PR is ready for review**, use your triage meeting to find someone to review it within a reasonable amount of time. If you cannot manage a review soon, explain that to the contributor so they’re not left hanging and know what to expect.

- **If a PR is ready for review**, find someone to review it within a reasonable amount of time. If you cannot manage a review soon, explain that to the contributor so they’re not left hanging and know what to expect.

twiss · 2024-01-04T16:54:34Z

MAINTAINERS.md

+npm version {major,minor,patch}
+```
+
+Which will trigger the `preversion`, `version` and `postversion` scripts specified in the `package.json`. These will handle the testing, building, publish to npm and pushing to GitHub. 


Suggested change

Which will trigger the `preversion`, `version` and `postversion` scripts specified in the `package.json`. These will handle the testing, building, publish to npm and pushing to GitHub.

(depending on whether a major, minor or patch release is to be created.) This command will run tests, create a build, publish it to npm, and push to GitHub.

AlbaHerrerias and others added 7 commits December 13, 2023 11:05

docs: add code of conduct

029d8ba

docs: update bug report template

5b5c2b6

docs: add pull request template

56a05e1

docs: add contributing guide

da2989d

Co-Authored-By: Alex Feyerke <391124+espy@users.noreply.github.com> Co-Authored-By: Ayham Kteash <11707377+ayhamthemayhem@users.noreply.github.com>

docs: add maintainers guide

21fcdc2

Co-Authored-By: Alex Feyerke <391124+espy@users.noreply.github.com>

docs: add new contributors guide

436b87d

Co-Authored-By: Alex Feyerke <391124+espy@users.noreply.github.com>

docs: update readme

43486c1

twiss reviewed Jan 4, 2024

View reviewed changes

hulkoba added 3 commits January 11, 2024 14:08

docs: address PR changes

089abc0

docs(test): remove detailled test instructions

5323148

docs(maintainers): remove unrealistic options

df7f674

larabr force-pushed the v6 branch 2 times, most recently from b340608 to b41298a Compare March 22, 2024 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[V6] Add contributing guides and improve documentation #1711

[V6] Add contributing guides and improve documentation #1711

AlbaHerrerias commented Dec 13, 2023

twiss left a comment

twiss Jan 4, 2024

twiss Jan 4, 2024

twiss Jan 4, 2024

twiss Jan 4, 2024

twiss Jan 4, 2024

twiss Jan 4, 2024

twiss Jan 4, 2024

twiss Jan 4, 2024

larabr Jan 8, 2024 •

edited

twiss Jan 8, 2024

twiss Jan 4, 2024

twiss Jan 4, 2024

	- Is this a security related bug? If so, disclose it privately following the procedure described in https://github.com/openpgpjs/openpgpjs/blob/main/SECURITY.md
	- Could this issue cause a security vulnerability? If so, please disclose it privately following the procedure described in https://github.com/openpgpjs/openpgpjs/blob/main/SECURITY.md

	<!-- Explain what this PR does and why we need it -->
	<!-- Explain what this PR does and why we need it -->


		6. Make sure to update or add to the tests when appropriate. Run the appropriate testing suites to check that all tests pass after you've made changes. You can read about our different types of tests in our [Testing](#testing) section.

		7. If you added or changed a feature, make sure to document it accordingly in the [README.md](https://github.com/openpgpjs/openpgpjs/blob/main/README.md) file.


		### Step 3: Assign the Work

		It should be clear who is responsible for working on an issue once it has been triaged and deemed actionable. This also includes eventual reviews. Ideally don’t assign things to people who aren’t expecting it.


		### Step 4: Follow up

		- If no PR is opened on an issue within a month, a maintainer should contact the assignee and ask them to create a PR or unassign themselves

	- If a PR is ready for review, use your triage meeting to find someone to review it within a reasonable amount of time. If you cannot manage a review soon, explain that to the contributor so they’re not left hanging and know what to expect.
	- If a PR is ready for review, find someone to review it within a reasonable amount of time. If you cannot manage a review soon, explain that to the contributor so they’re not left hanging and know what to expect.

	Which will trigger the `preversion`, `version` and `postversion` scripts specified in the `package.json`. These will handle the testing, building, publish to npm and pushing to GitHub.
	(depending on whether a major, minor or patch release is to be created.) This command will run tests, create a build, publish it to npm, and push to GitHub.

[V6] Add contributing guides and improve documentation #1711

Are you sure you want to change the base?

[V6] Add contributing guides and improve documentation #1711

Conversation

AlbaHerrerias commented Dec 13, 2023

twiss 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

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

larabr Jan 8, 2024 • edited

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

larabr Jan 8, 2024 •

edited