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

doc: Update Developer Guide (contributing information) #395

Merged
merged 5 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added source/_images/devguide_branch_conflicts.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/devguide_failed_cla_check.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/devguide_out_of_date_branch.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/devguide_passing_checks.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/devguide_pr_status.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/devguide_workflow_approval.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_backport.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_blocked_by_other_work.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_changes_requested.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_closed-inactivity.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_core_contributor.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_duplicate.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_inactive.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_needs_test_run.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_product_review.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added source/_images/pr_label_waiting_on_author.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions source/developers/references/developer_guide/links.rst
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@
.. _Accessibility Guidelines: http://edx.readthedocs.io/projects/edx-developer-guide/en/latest/accessibility.html
.. _Analytics Guidelines: http://edx.readthedocs.io/projects/edx-developer-guide/en/latest/analytics.html
.. _Eventing Design and Review Process: https://openedx.atlassian.net/wiki/display/AN/Eventing+Design+and+Review+Process
.. _Product Review Process: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3875962884/DRAFT+How+to+submit+an+open+source+contribution+for+Product+Review

.. _Durham University Alt Text Checker: https://www.dur.ac.uk/cis/web/accessibility/tools/alttext/

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
######################################################
Copy link
Contributor Author

@sarina sarina Nov 8, 2023

Choose a reason for hiding this comment

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

This is a new file, written by Michelle and Tim, and reviewed/transcribed by me.

Overview of Review Process for Community Contributions
######################################################

Community contributions, in the form of pull requests, go through a process
shepherded by the `Community Contribution Project Managers`_. These
contributions are sometimes referred to as "OSPRs" (open source pull requests).

See the :doc:`pull-request-statuses` if you're looking for more information about
what the various statuses and labels of your pull request (PR) mean.

.. contents::
:local:
:depth: 1

=======================================
Community Contribution Project Managers
=======================================

The main goal of the `Community Contribution Project Managers`_ is to triage all
pull requests that come in from contributors across the community, and help
those PRs get through to being merged. During the triage and review process, the
Community PMs are responsible for:

* Welcoming new contributors to the community

* Triaging community pull requests in a timely manner

* For new contributors, ensuring they submit a CLA form (if contributing as an
individual), or get added to their organization's entity agreement (if
contributing on behalf of their organization)

* Ensuring the correct amount of detail is provided for the requested change(s)
(e.g. if there is not context or description for your contribution, we'll ask
you to provide one)

* Making sure checks are able to run on the pull request

* Routing pull requests to the appropriate reviewers / `maintainers`_, and
making sure they're addressed in a timely manner

* Connecting contributors with the appropriate people if troubleshooting is
needed, or if there are technical questions

* Helping to get the pull request merged once everything is approved

At a high level, the pull request process is as follows:

#. Initial triage on a new pull request is done by the Community PMs within a few days

#. Note: some pull requests require review by our Product team. Please see
:doc:`contributor` for requirements needed for the Product team to
properly review your contribution. Ideally, this would be done before code
is written.

#. If checks need to be enabled, or a CLA form needs to be submitted, the
Community PMs will help get checks running and help with any questions
regarding the CLA

.. image:: /_images/devguide_workflow_approval.png

.. image:: /_images/devguide_failed_cla_check.png

#. If there are checks failing on your pull request, you'll need to look into
what's causing the issue. The Community PM can direct you to someone to help
if needed.

#. Note: please see :doc:`../../running_pr_tests` that outlines common issues
that pop up while running tests

#. Once everything is up and running, the Community PMs are available to help
with any questions that come up while product and/or engineering review is
in-progress

#. If the pull request review process takes a bit longer than expected, you may
notice the following alerts pop up, and the Community Project Managers might
ask you to take some action:

#. **“This branch is out-of-date with the base branch”** which means you'll
need to rebase your changes.

.. image:: /_images/devguide_out_of_date_branch.png

#. **“This branch has conflicts that must be resolved”** which means someone
has changed information in the same file on the target branch of the PR
(which is usually the master). You will need to reconcile those changes
with your own.

.. image:: /_images/devguide_branch_conflicts.png

#. When all checks are green, tests are passing, there are no conflicts, and the
work is ready for engineering review, the Community PMs will assign the
appropriate reviewer / maintainer to take a look

.. image:: /_images/devguide_passing_checks.png

#. During review, you may receive feedback or change requests that require
action on your part.

#. Once all feedback and changes have been incorporated, the pull request will
be able to be merged by the reviewer / maintainer.

As pull requests move through the process, the Community Project Managers update
the PR's status, and use labels to denote different aspects / states of the PR.
This helps reviewers better understand what action(s) are needed, and by who.

A breakdown of these statuses and labels can be found at the
:doc:`pull-request-statuses` for reference (though, PR authors are not
responsible for updating PR status or labels).

On the right-hand side of the PR, you'll be able to see the status of your pull
request, the Community PM responsible for the PR, and the last date the
Community PM checked in on it. To do this, find the "Projects" section, and
click on the "⌄" caret in the top right corner next to "Contributions" to see a
display like this:

.. image:: /_images/devguide_pr_status.png
sarina marked this conversation as resolved.
Show resolved Hide resolved

============================
FAQs about your Pull Request
============================

**My pull request has been waiting for review for a few weeks, is something
wrong?**

No! We receive hundreds of pull requests from the community (which is awesome!),
but that means sometimes the process to get PRs through the review process and
merged might take a bit of time due to things like:

* The reviewing team is at capacity and needs to review the pull request during a later sprint
* Related work or dependencies may be blocking a PR's ability to be merged

The Community PMs will do our best to make sure all updates related to delays
are communicated within the PR. Additionally, if the reviewing team needs
additional information or context for your contribution, they will ask within
the PR.

**There haven't been any updates in my PR in a few weeks. How do you handle
stalled PRs?**

The Community Project Managers proactively stay on top of pull requests, and
will ping reviewers for updates, actions, etc. when things seem stalled. If you
have any questions, you can tag the assigned Community PM for help / updates in
the PR at any time.

**I submitted a PR a while ago, something came up, and I am not able to work on
it for the foreseeable future. What happens now?**

If a PR is stalled on the author's side, the Community PMs will ping the author
a few times before labeling the PR as “inactive” after a month or so.

* If you want to pursue the PR, but don't have capacity at the moment, please
let us know directly in the PR if possible. At that point we can leave it as a
draft, or we can close it if needed, and can reopen it at a later time if
you're able to pursue it.
* If an author is unresponsive for a month or more, or hasn't had capacity to
come back to check on the PR, the Community PMs may need to close the PR due
to inactivity (though, again, it can be reopened at a later time if needed).

**My PR was labeled as “blocked” - what does that mean?**

Occasionally, a pull request may get blocked by other on-going or related work.
The Community PMs will stay on top of it to make sure it gets unblocked as soon
as possible, but note that it may take a bit depending on what's blocking it.
Updates from the Community PMs and reviewing team should be made in the PR so
you have updated information.

**My PR was closed and not merged - why?**

There may be times where a pull request is closed (instead of merged) by an
owning team because the update may no longer be needed, it's duplicative of
other work, or other reasons. The reason for closing the pull request should be
Comment on lines +170 to +172
Copy link

Choose a reason for hiding this comment

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

@sarina That reminds me -- I think the "duplicate" label is missing from the section on Contribution Board Labels.

Copy link
Contributor Author

@sarina sarina Nov 20, 2023

Choose a reason for hiding this comment

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

Can you provide a description for "duplicate"?

I used the one from the label on GitHub - "This issue or pull request already exists elsewhere."

made clear to you in the PR, and if it's not, the Community PM assigned to your
pull request can help you get that information.



.. _Community Contribution Project Managers: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3548807177/Community+Contributions+Project+Manager
.. _maintainers: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3426844690/Maintainership+Pilot
130 changes: 77 additions & 53 deletions source/developers/references/developer_guide/process/contributor.rst
Original file line number Diff line number Diff line change
@@ -1,47 +1,75 @@
.. _Contributor:

************
Contributing
************

Contributing your custom code to the Open edX project is a two-step process:
sarina marked this conversation as resolved.
Show resolved Hide resolved
review of your idea by the product team (the Product Review Process),
followed by a review of your code by the project's core committers.
We describe this process on the `Product Review Process`_ wiki page.

Note::

By and large, bug fixes do not require the Product Review Process.
However, if your bug fix will change user facing behavior, you should
check with the Product Working Group on the best way to land your fix
before beginning to write code.

----------------------------------------
Product Review Process & Product Roadmap
----------------------------------------

New features added to the Open edX project go through a `Product Review
Process`_, lead by the `Open edX Product Working Group`_. If you're interested
in adding new features, we ask that you undergo the product review process
_before_ you begin coding your feature. This allows the project to get
alignment on new features and help guide you to implement the feature in
a way that works with the overall platform. Additionally, there might well
be someone else already working on the same change you want to make,
and it's much better to collaborate than to submit incompatible pull requests.

------------------
Finding Work To Do
------------------
If you open up your feature request with a solid description, the product owners
will be able to quickly understand your change and prioritize it for
review. However, they may have some questions about your intention, need,
and/or approach that they will ask about.

TODO: Add Content Here (how to use Feature Requests & Product Roadmap)
To read more about the product review process and how to submit your idea,
please visit the `Product Review Process`_ wiki page.

Before you make a pull request, it's a good idea to reach out to the
Open edX community to discuss your ideas. There
might well be someone else already working on the same change you want to make,
and it's much better to collaborate than to submit incompatible pull requests.
The `Community Discussions`_ page on the openedx.org website lists different
ways you can ask, and answer, questions. The earlier you start the
conversation, the easier it will be to make sure that everyone's on the right
track.
Features that the project wants, as well as ones being currently worked on
by community members, are represented on the `Product Roadmap`_. This
kanban-style board represents units of work on individual cards. Check
out the tickets on the far left for help reading the board. The tabs at
the top will allow you to drill into roadmap tickets by platform area.

.. _Community Discussions: https://openedx.org/community/connect/
If you're interested in contributing to the Open edX project but don't know
what to contribute, check out the roadmap! If you're interested in picking
up or collaborating on a ticket, join the conversation by adding a comment
on the ticket.

It's also sometimes useful to submit a pull request even before the code is
working properly, to make it easier to collect early feedback. To indicate to
others that your pull request is not yet in a functional state, just prefix the
pull request title with "(WIP)" (Work In Progress), or start the pull request
as a draft on GitHub. Please include a link to a WIP pull request in any
discussion threads you start.
----------------------------
Getting Preliminary Feedback
----------------------------

It is sometimes useful to submit a pull request even before the code is
working properly, to make it easier to collect early feedback on your
implementation. To indicate to others that your pull request is not yet in a
functional state, just prefix the pull request title with "(WIP)" (Work In
Progress), and start the pull request as a draft on GitHub.

----------------
Landing The Work
----------------
Please include a link to your roadmap ticket in the PR description, and add a
link to a WIP pull request in any discussion threads you start.

TODO: Update This Section
-----------------
Landing Your Work
-----------------

Once you're ready to submit your changes in a pull request, check the following
Once you're ready to submit your code changes in a pull request, check the following
list of requirements to be sure that your pull request is ready to be reviewed:

#. Prepare a detailed pull request description. When you open
your pull request, put your cover letter into the "Description" field on
GitHub.
#. Prepare a detailed pull request description. Most repositories have a template
to be followed when you open the PR in the GitHub UI. Please fill out that template
with as much detail as possible, including links to any roadmap tickets or
forum discussions as well as screenshots or screencasts.

#. The code should be clear and understandable. Comments in code, detailed
docstrings, and good variable naming conventions are expected. See the
Expand All @@ -65,7 +93,7 @@ list of requirements to be sure that your pull request is ready to be reviewed:
request, for forensic purposes when trying to diagnose a regression
or understand a bug.

#. All code in the pull request must be compatible with Open edX's AGPL
#. All code in the pull request must be compatible with the Open edX project's AGPL
license. This means that the author of the pull request must sign a
`contributor's agreement`_, and all libraries included or
referenced in the pull request must have `compatible licenses`_.
Expand All @@ -88,19 +116,18 @@ list of requirements to be sure that your pull request is ready to be reviewed:

#. For pull requests that make changes to the user interface, please include
screenshots of what you changed. GitHub will allow you to upload images
directly from your computer. In the future, the core committers will produce
a style guide that contains more requirements around how pages should appear
and how front-end code should be structured.
directly from your computer. Changes should only use elements from the
`Paragon pattern library`_.

#. The pull request should contain some documentation for the feature or bug
fix, either in a README file or in a comment on the pull request. A well-
written description for the pull request may be sufficient.
fix. A well-written description for the pull request is often sufficient.

#. The pull request should integrate with existing infrastructure as much as
possible, rather than reinventing the wheel. In a project as large as Open
edX, there are many foundational components that might be hard to find, but
it is important not to duplicate functionality, even if small, that already
exists.
possible, rather than reinventing the wheel. In a project as large as ours,
there are many foundational components that might be hard to find.
It is important not to duplicate functionality, even if small, that already
exists. Posting in the `forums`_ is an easy way to find out if functionality
you're looking for already exists.

#. The author of the pull request should be receptive to feedback and
constructive criticism. The pull request will not be accepted until all
Expand All @@ -111,13 +138,17 @@ list of requirements to be sure that your pull request is ready to be reviewed:
comment explaining why the contributor has chosen not make any change based
on that feedback.

While your pull request is open, you may have questions about how things are
going, or how to check in on the progress of your review. We've prepared a
:doc:`FAQ-about-pull-requests` that should answer most of your questions.

It's also important to realize that you and the core committers may have
different ideas of what is important in the codebase. The power and freedom of
open source software comes from the fact that you can fork our software and
make any modifications that you like, without permission from us. However, the
make any modifications that you like without permission from us. However, the
core contributors are similarly empowered and free to decide what modifications
to pull in from other contributors, and what not to pull in. While your code
might work great for you on a small installation, it might not work as well on
might work great for you on your installation, it might not work as well on
a large installation, have problems with performance or security, not be
compatible with internationalization or accessibility guidelines, and so on.
There are many, many reasons why the core committers may decide not to accept
Expand All @@ -126,18 +157,6 @@ code change. However, if we do reject your pull request, we will explain why we
aren't taking it, and try to suggest other ways that you can accomplish the
same result in a way that we will accept.

Once A PR is Open
-----------------

If you open up your pull request with a solid description, the product owners
will be able to quickly understand your change and prioritize it for
review. However, they may have some questions about your intention, need,
and/or approach that they will ask about. A community
manager will ping you on GitHub to clarify these questions if they arise.

Once the product team has sent your pull request to the engineering teams for
review, all technical discussion regarding your change will occur on GitHub,
inline with your code.

Further Information
-------------------
Expand All @@ -156,3 +175,8 @@ following links:
.. _contributor's agreement: http://openedx.org/cla
.. _compatible licenses: https://openedx.org/open-edx-licensing
.. _OEP-51\: Conventional Commits: https://open-edx-proposals.readthedocs.io/en/latest/best-practices/oep-0051-bp-conventional-commits.html
.. _Paragon pattern library: https://paragon-openedx.netlify.app/
.. _forums: https://discuss.openedx.org/
.. _Product Review Process: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3875962884/DRAFT+How+to+submit+an+open+source+contribution+for+Product+Review
.. _Open edX Product Working Group: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3449028609/Product+Working+Group
.. _Product Roadmap: https://github.com/orgs/openedx/projects/4
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ other community members, and are generally polite and welcoming.
Core Contributors that write code gain write access and even maintainership to one or
more GitHub repositories in the openedx/ GitHub organization. If you're interested in
becoming a Core Contributor, the following resources may be helpful to you. You can
also find us on the `Open edX Slack <https://openedx.org/slack>` in the #core-contributors
also find us on the `Open edX Slack <https://openedx.org/slack>`_ in the #core-contributors
Slack room.

---------
=========
Resources
---------
=========

* `Core Contributor program homepage <https://openedx.atlassian.net/wiki/spaces/COMM/pages/3143205354/Core+Contributor+Program>`_
* `Open edX Proposal 54 <https://open-edx-proposals.readthedocs.io/en/latest/processes/oep-0054-core-contributors.html>`_, which defines the Core Contributor Program
Expand Down
Loading