Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Maintainer docs on "good first" and "help wanted" issues #3889

Merged
merged 14 commits into from
Apr 8, 2024
Merged
5 changes: 5 additions & 0 deletions documentation/general/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ issue be assigned to you when you're ready to work on it.
- [Good first issues](https://github.com/WordPress/openverse/issues?q=is:issue+is:open+sort:updated-desc+label:%22good+first+issue%22)
- [Issues wanting help](https://github.com/WordPress/openverse/issues?q=is:issue+is:open+sort:updated-desc+label:%22help+wanted%22)

It is recommended that first-time contributors begin with a "good first issue".
For more information on how Openverse maintainers think about "good first
issues", please see
[our additional documentation](/meta/contribution/good_first_and_help_wanted_issues.md).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The audience of the linked document is a bit mixed, if we include it here. Is it for maintainers or for contributors? Moving the definition of a good first issue to a separate document from the one that covers how maintainers help contributors complete their early contributions (predefs, meta-thoughts about good/bad Openverse domains for good-first-issues, etc).

Some version of the "What makes an issue a good first issue?" section makes sense to have as an all-audience context. How to write the good first issue, on the other hand, is just for maintainers. A longer document might discourage contributors looking for something quick from reading it, potentially having them miss out on the context.

As in the original discussion, I think NeoMutt's separation makes a lot of sense, and has layers to "ease" someone into understanding the different levels of possible contribution.

Here's the definition of "Easy Coding Tasks", linked from the "Newbie Tutorial". Ignoring the list of specific tasks in that definition, the definition itself is really straightforward, and encapsulates the philosophy behind it. It's good to have a deeper explanation for maintainers to wrap their heads around, especially because we're also investing time in writing good descriptions for these tasks. For contributors, however, a higher-level description of the expectations they should have as coder and that we will have as reviewer/maintainer, would be faster to incorporate.

All of this is just to share an example for why I think separating the definition of "good first issue" for a primary audience of the contributor, rather than linking to the maintainer oriented definition, is important. I hope the example shows an effective way to layer this. I don't know if NeoMutt have deeper documentation elsewhere for their maintainers regarding this, but their docs are significantly more concise for new contributors than ours are to begin with. I think that's a good model, because if it's too long or looks too involved, people won't read it and fewer will be better off.

At least these three points (we don't have the mentorship ability) are true for us, and say things in a language a contributor would understand quickly:

Simple – You don’t need to be a C master to do them
Understandable – You don’t need to know the insides of NeoMutt
Low priority – If it takes you a while, it doesn’t matter

For Openverse, maybe:

Simple - You don't need to be a JavaScript or Python master to do them
Understandable - Expectations are clearly documented in the issue and you don't need to learn everything about Openverse to complete them
Low priority - If it takes you a while, it doesn't matter

I can't think of a better way to describe low priority 🙂

Another caveat for maintainers: good first issue shouldn't be part of a project stream at all, probably! Maybe only very small bugs or quality-of-life improvements. Unfortunately, we have very little feature related work that would be "good first issue" under that definition, though.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also realising here that NeoMutt explicitly says C master, implying, you need to know something about C to begin with. I wonder if we need to set that kind of bar as well. Like for each app, we would want to say something like:

  • You should be comfortable with Python/TypeScript
  • You should be familiar with the basic concepts for Django/Vue/Airflow
  • etc.

Is the expectation that a good first issue is a way to learn about the technologies, or about Openverse? That's probably a good thing to clarify: what is our intention for someone completing good first issues? If we intend contributions to be focused on both, is there a baseline expectation for where someone is? Do we expect to need to leave feedback on a PR regarding the basics of JavaScript/Python (is vs == in Python, == vs === in JS, for example), or should contributors, even first time contributors, already have that late-beginner/early-intermediate level of understanding about those languages (acknowledging there are sometimes complex edge cases that even advanced users of those languages struggle with)?

This is a bit of target-audience analysis that our issues could benefit from, and help narrow the scope of what details we need to include in a good first issue for them to be considered complete and appropriately detailed for the level of experience we anticipate and hope new Openverse contributors bring.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now I am going to remove this reference from this file. Your comments are fantastic so I'll make sure to reference them in a follow-up issue on documenting good first issues for contributors rather than maintainers.

Concerning this question:

Is the expectation that a good first issue is a way to learn about the technologies, or about Openverse?

I think the later, primarily learning about Openverse. Refining one's understanding of different technologies can be an aspect of writing any code, but I do not think it is an explicit goal we need to promote. Ideally, issues are documented clearly enough that contributors can gauge if their current skillset is sufficient to cover the issue, or if they're nearly there and can fill any gaps on-the-fly.

Do we expect to need to leave feedback on a PR regarding the basics of JavaScript/Python (is vs == in Python, == vs === in JS, for example), or should contributors, even first time contributors, already have that late-beginner/early-intermediate level of understanding about those languages (acknowledging there are sometimes complex edge cases that even advanced users of those languages struggle with)?

IMO our contributors should have this level of late-beginner/early-intermediate knowledge. If they do not it's likely a PR will have significant issues and either be closed or taken over by a maintainer. I don't think we should spend time walking people through these types of learnings.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, a separate issue makes sense. I share your opinion on what appropriate expectations would be for our issues, and in particular think it aligns well with the time and focus we are able to dedicate and explicitly commit to for any PR review or contributions in general.


## Non-code contributions

If programming is not your cup of tea, there are ways to contribute to Openverse
Expand Down
194 changes: 194 additions & 0 deletions documentation/meta/contribution/good_first_and_help_wanted_issues.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
# Maintainer guidelines for "Good first" and "Help wanted" issues

GitHub issues tagged "good first issue" and "help wanted" are often a
contributors entry-point into a project. As such, it is important that these
issues not only are true to their tag names, but that they are also giving new
contributors an ideal first experience. Finally, _providing_ this experience
needs to be straightforward and repeatable for maintainers.

Here you will find guidelines, predefined responses, and tools for creating
these issues and guiding the contributors who seek to resolve them.

> **Note**: Looking for quick information to deal with an open issue? See:
>
> - [Predefined responses](#reply-templates-and-behaviors)
> - [Timing and process](#timing-and-process)
> - [Good first issue template boilerplate](#additional-boilerplate-for-good-first-issue-templates)

## What makes an issue a good first issue?

Generally speaking, a good first issue is one that provides a contributor an
isolated, discrete task that teaches the contributor something new about
Openverse. This learning enables the contributor to take on subsequent issues
with increased confidence.

Here are some example signs that an issue is sufficiently isolated. These
issues:

- Do not require a holistic understanding of Openverse's ingestion pipeline
- Are limited to single programming language or programming paradigm (i.e,
frontend JS, CSS, and markup)
- Occupy a single slice of the Openverse stack
- Touch a single file or as few files as necessary; ideally these files are
colocated in the repository
- Are based on the `main` branch and not sub-features of a larger, multi-PR
changeset

Other qualities of appropriate "good first" issues are that they:

- Are not 'high priority', 'critical priority', or otherwise time sensitive
- Are appropriate for those new to open source or Git workflows in-general
- Can be, approximately, completed within two hours, one of which may be
necessary for local development environment setup

## Writing good first issue descriptions

Good first issue descriptions should provide as much context as possible. These
issues should generally be self-contained; all the documentation necessary to
complete the issue should be included with or linked to in the issue
description. The aim is to make the issue as approachable as possible while
setting clear expectations. This up-front effort is to the benefit of
contributors and maintainers. Ideally, it decreases the amount of time
maintainers spend reviewing PRs that have not met the minimum requirements
(e.g., no unit tests, linting failures, and so on). For contributors, it helps
prevent them from feeling discouraged after completing the "core functionality"
of a PR to then have to revisit the PR and add significant changes.

Descriptions should:

- Clearly articulate the expected outcome of resolving the issue. Include any of
the following, when relevant:
- Changes (modifications or additions) to tests
- Link to instructions for updating Playwright snapshots if the change might
require it
- Changes (modifications or additions) to documentation
- Link to the relevant files and/or lines of code which need changing
- Reference any past PRs which will help the contributor, for example:
- A PR with a similar change
- A PR which originally implemented the feature being modified
- Relevant Openverse domain knowledge, either links to succinct documentation or
links and a summary of the important aspects when documentation is detailed
- Ideally "good first issue"s require essentially no Openverse-specific domain
knowledge, but if they at all do (for example, to explain why a non-obvious
solution is requested) then it must be included
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think adding these headings to all of our issue templates would make the templates too annoying? We could comment out the headings so that if the person writing the issue does not need them, they can simply leave the unnecessary parts as comments.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, also, I think we should add "Please comment below if you want to work on this issue" to the bottom of the description. Otherwise, if a contributor has never interacted with the issue, we cannot assign it to them.


zackkrida marked this conversation as resolved.
Show resolved Hide resolved
### Additional boilerplate for good first issue templates

In addition to our standard issue template, good first issues should _also_
contain the following block of requirements:

```md
## Good first issue checks

- Have I filled out the PR template correctly?
- Did I include testing instructions?
- Does my PR pass linting? (Test either via precommit or by running `just lint`
manually)
- Do all the tests still pass?
- If JavaScript changes, run `pnpm -r run test`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just frontend/run test should work here (for consistency with the other commands)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with just frontend/run test is that it only runs frontend tests. pnpm -r run test runs test for all workspaces, which will include the eslint plugin, which we've had contributors open PRs for. The test suites run parallel to each other, so it doesn't add any extra time, and prevents any chance of someone getting confused about where the changes they made are relevant. Especially important if we have anything in packages in the future that is depended on by the frontend and not explicitly tested in the frontend suite (which makes sense, we wouldn't test dependencies normally). Using pnpm -r run test here (and ideally everywhere) prevents it from being overlooked in the future.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for explaining!

- If API changes, run `just api/test`
- If catalog changes, run `just catalog/test`
```

## On "help wanted" issues

All good first issues are naturally "help wanted" issues as well. "Help wanted"
issues which do not also include the "good first issue" label are excellent
candidates for second, third, fourth, and so on issues from repeat contributors.
If a contributor is labeled as a "first time contributor" in the GitHub user
interface, make sure the issue they are working on is indeed marked "good first
issue", or that it is otherwise clear that they have advanced knowledge of the
issue's problem space that makes them an appropriate candidate.

For example, the maintainer of a 3rd party library we use might see we are
having an issue with their code and offer a PR with a fix. This would be
appropriate given their expertise.

## Timing and process

It can often be tricky to determine how and when to make requests of a community
contributor. The following table provides guidelines for specific scenarios.

Response times may be from the contributor _or_ from a maintainer depending on
the situation.

| Scenario | Recommended Response Time (Days) |
| -------------------------------------------- | ---------------------------------------------------------------- |
| New PR submitted | Typically priority based, but for community PRs should be 3 days |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update the PR review reminder bot to check the PR author, and use a 3 day expectation for anyone outside the maintainers list?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great idea! Probably best to do it in this PR rather than follow-up, so I'll try and add that asap.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, given my AFK, I'll make a separate issue for this. It requires adding a new method to our GitHubApi class in the catalog and making small but non-trivial changes to the pr review reminder dag that I unfortunately don't have time for.

| Maintainer has pinged PR author | 3-5 |
| Maintainer follow-up on previous PR feedback | 1-2 |
| PR is ready for merge | 1-2 |

## Reply templates and behaviors

These predefined responses ("predefs") provide consistient solutions to common
situations.

If you find yourself dealing with a recurring scenario that _isn't_ included in
this list, please submit a pull request to add it here.

### Scenario 1: Issue Request

A "first time contributor" asks if they can work on an issue.

#### Initial Response

```md
Hi `@user`, thank you for your interest in contributing to Openverse! I've
assigned this issue to you. If you have any questions, you may leave them here.

Please check out our
[welcome](https://docs.openverse.org/general/contributing.html) and
[quickstart](https://docs.openverse.org/general/quickstart.html) documentation
pages for getting started with setting up your local environment.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might also be good to include a prompt for maintainers to share the particular "apps" quickstarts the contributor will need to keep in mind. Something like "Pay particular attention to the quickstarts linked for API, as that is where the changes need to be implemented, and you need to run the API locally to verify your changes".

```

#### Follow up

Assign the issue to the user. Check on progress of the issue as part of our
regular prioritization process. If the contributor doesn't reply or create the
PR, move on to [scenario #3](#scenario-3-absent-contributor).

### Scenario 2: Improper PR Template

A community pull request author did not use the pull request template or failed
to fill out all sections correctly.

#### Initial Response

```md
Hi `@user`, could you update your pr description to use the
zackkrida marked this conversation as resolved.
Show resolved Hide resolved
[pull request template](https://github.com/WordPress/openverse/blob/main/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md)?
If you have any questions please let us know in the comments.
```

#### Follow through

If the contributor doesn't reply or update the PR, move on to
[scenario #3](#scenario-3-absent-contributor). If they update the template,
proceed. If they ask for help or have other concerns, update the template for
them.
zackkrida marked this conversation as resolved.
Show resolved Hide resolved

### Scenario 3: Absent Contributor

A contributor opened a pull request but hasn't updated it or responded to
changes in the required timeframe.

#### Initial Response

```md
Hi {@user}, are you still able to work on this PR? We appreciate all the work
you have completed so far. If you are not able to finish this pull request we
can unassign you, so a maintainer can take over the remaining work.
```

#### Follow through

Wait 5 business days for the user to respond. If they do not respond, unassign
the PR, draft the PR, and reply with the following:

```md
@{user} thank you again for your efforts here. I have unassigned this PR and
drafted it to be picked up by a maintainer when available. If you would ever
like to resume work, do not hesitate to let us know here.
```
1 change: 1 addition & 0 deletions documentation/meta/contribution/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ communication_aliases
becoming_a_committer
maintainer_tasks
codespell
good_first_and_help_wanted_issues
```
Loading