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

chore: cleanup #646

Merged
merged 5 commits into from
Nov 14, 2024
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
13 changes: 7 additions & 6 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ View the published list of `Open edX Proposals (OEPs)`_ on ReadTheDocs.
Getting Started
***************

This repository holds a bunch of text files, each of which represent an OEP or
This repository holds a bunch of text files, each of which represents an OEP or
supplementary documentation. For browsing these OEPs, we recommend you read them
in `their published form <https://open-edx-proposals.readthedocs.io>`_ on ReadTheDocs.

Expand All @@ -43,20 +43,19 @@ all proposed changes and additions on `pull requests`_.
.. _ReStructured Text (RST): https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html
.. _pull requests: https://docs.openedx.org/en/latest/developers/quickstarts/first_openedx_pr.html



Testing Locally
***************

Before you make a pull request with your proposed changes, please try to visually test your changes first.

To test locally in a Python virtual env, you will first need to install `GraphViz <http://graphviz.org/>`_
On a Mac, this can be done via ``brew install graphviz``; on Ubuntu, use ``sudo apt install graphviz``; on Red Hat variants use ``sudo dnf install graphviz``.
Next run the following commands::
Next, run the following commands

.. code-block:: bash

pip install sphinx # it may fail for non-obvious reasons without this
make requirements

make html

Common Warnings
Expand All @@ -67,7 +66,9 @@ Document isn't Included in any toctree

If you have some documents that you only reference via ``:doc:`` or ``:ref:`` tags you may get this error.
If there is no table of contents that the files obviously belong in, an easy way to fix this error is to put the
documents in a hidden toctree near where they are referenced::
documents in a hidden toctree near where they are referenced:

.. code-block:: rst

.. toctree::
:hidden:
Expand Down
2 changes: 1 addition & 1 deletion oeps/best-practices/oep-0017-bp-feature-toggles.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
OEP-17: Feature Toggles
#########################
#######################

.. list-table::
:widths: 25 75
Expand Down
2 changes: 1 addition & 1 deletion oeps/best-practices/oep-0018-bp-python-dependencies.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
OEP-18: Python Dependency Management
######################################
####################################

+-----------------+--------------------------------------------------------+
| OEP | :doc:`OEP-18 <oep-0018-bp-python-dependencies>` |
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
OEP-19: Developer Documentation
#################################
###############################

.. list-table::
:widths: 25 75
Expand Down
2 changes: 1 addition & 1 deletion oeps/best-practices/oep-0022-bp-django-caches.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
OEP-22: Caching in Django
###########################
#########################

+-----------------+--------------------------------------------------------+
| OEP | :doc:`OEP-22 <oep-0022-bp-django-caches>` |
Expand Down
2 changes: 1 addition & 1 deletion oeps/best-practices/oep-0037-bp-test-data.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
OEP-37: Dev Data
##################
################

.. list-table::
:widths: 25 75
Expand Down
2 changes: 1 addition & 1 deletion oeps/best-practices/oep-0038-Data-Modeling.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
OEP-38: Data Modeling Best Practices
######################################
#####################################

+---------------+------------------------------------------------------------+
| OEP | :doc:`OEP-0038 <oep-0038-Data-Modeling>` |
Expand Down
2 changes: 1 addition & 1 deletion oeps/best-practices/oep-0049-django-app-patterns.rst
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ Example
from enum import Enum

from attrs import field, frozen, validators

class ProgramStatus(Enum):
ACTIVE = "active"
RETIRED = "retired"
Expand Down
188 changes: 94 additions & 94 deletions oeps/best-practices/oep-0066-bp-authorization.rst

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions oeps/best-practices/oep-0067-bp-tools-and-technology.rst
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ This technology is not specific to frontend or backend code.

**Rationale**: It is important to measure the amount of code covered by our
automated test suites. By striving for a high level of test coverage, we can
reduce the number of bugs that can only be found via manual testing,
reduce the number of bugs that can only be found via manual testing,
and by using codecov to run coverage in CI,
contributors are automatically reminded to include tests for any new code.

Expand All @@ -84,12 +84,12 @@ This technology is not specific to frontend or backend code.
Frontend Technology Selection
=============================

.. note::
.. note::

The phrase “frontend” is used to mean any part of the platform that is shown to
users. This encompasses views rendered in Python on the server, interactive
interfaces written using JavaScript, and CSS styling.

.. _Use React:

#. **Use React**
Expand Down Expand Up @@ -250,7 +250,7 @@ Frontend Technology Selection

**Rationale**: It is important for users of the Open edX platform that we deliver
reasonably sized JavaScript bundles. This provides faster load times to all users,
and is vital for users with low bandwidth and/or metered connections. To ensure we
and is vital for users with low bandwidth and/or metered connections. To ensure we
don't unintentionally increase the size of our JavaScript bundles, we utilize BundleWatch
for automated bundle size monitoring.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ Rejected Alternatives
Coveralls
=========

Coveralls has been historically used for tracking coverage but the service has become less reliable over time and the larger python has moved away from it.
Coveralls has been historically used for tracking coverage but the service has become less reliable over time and the larger python has moved away from it.

.. _Codecov: https://about.codecov.io/
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ Context
*******

It is important for users of the Open edX platform that we deliver reasonably sized JavaScript bundles. This provides
faster load times to all users, and is vital for users with low bandwidth and/or metered connections.
faster load times to all users, and is vital for users with low bandwidth and/or metered connections.

It is currently possible to manually monitor bundle size information by utilizing webpack configurations provided by
frontend-build, but this functionality is not very discoverable. Given the "hidden" nature of this functionality, it is
very possible for changes that increase bundle size to be missed in the PR review process.
very possible for changes that increase bundle size to be missed in the PR review process.

The increased visibility provided by automated bundle size monitoring will ensure we don't unintentionally increase
the size of our JavaScript bundles, as well as encourage maintainers to adopt best practices such as `code splitting`_.
Expand All @@ -28,7 +28,7 @@ decide if this CI check will be blocking or informative on a repo-by-repo basis.
BundleWatch
===========

`BundleWatch`_ is solely focused on ensuring bundle sizes stay under control. It is actively maintained and used by popular projects such as `bootstrap`_.
`BundleWatch`_ is solely focused on ensuring bundle sizes stay under control. It is actively maintained and used by popular projects such as `bootstrap`_.

Consequence
***********
Expand All @@ -41,13 +41,13 @@ Rejected Alternatives
*********************

1. **Code Checks**

`Code Checks`_ is a pluggable framework for automated code review. It provides more
than just bundle size monitoring, presenting us with an opportunity to rethink our
current CI workflows. For example, Code Checks provides test coverage monitoring,
which would allow us to re-evaluate the choice to use CodeCov as documented in
:doc:`0006-codecov`.

While this additional functionality is potentially desirable, moving forward with a
single purpose tool (`BundleWatch`_) provides us the functionality we need with
minimal changes to our current CI workflows.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ Accepted
Context
*******

As documented in :doc:`OEP-0011-0004 <0004-js-ecma>`, the Open edX community has previously
:ref:`decided against adopting TypeScript <Reasons for rejecting TypeScript>`. In the time
As documented in :doc:`OEP-0011-0004 <0004-js-ecma>`, the Open edX community has previously
:ref:`decided against adopting TypeScript <Reasons for rejecting TypeScript>`. In the time
since that decision was made, TypeScript has grown to be quite popular and commonplace for JavaScript developers.
This has led to more discussion within the community about the `pros and cons of adopting TypeScript`_.

Expand All @@ -33,7 +33,7 @@ Rejected Alternatives
*********************

1. **Continue without TypeScript**

It is possible to use other tools to add some of the functionality TypeScript provides
to JavaScript code. However, considering the main point of opposition to TypeScript adoption
was keeping a low barrier of entry for frontend developers, this could be counterproductive.
Expand All @@ -42,6 +42,6 @@ Rejected Alternatives

It would be possible to convert all JavaScript throughout the Open edX project (or any given
repository within the project) to TypeScript. It is not clear what the benefit of doing so would
be.
be.

.. _pros and cons of adopting TypeScript: https://github.com/openedx/paragon/discussions/1186
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ Rejected Alternatives
for identifying the root cause of a test failure but resulted in a
pretty high volume of PRs that needed to be reviewed (especially in
repositories which normally don't need to be updated very often). Renovate has
`configuration options to set the frequency of checking for new dependency
`configuration options to set the frequency of checking for new dependency
releases <https://renovatebot.com/docs/noise-reduction/#scheduling-renovate>`_,
and to allow related packages to be `updated in the same pull
and to allow related packages to be `updated in the same pull
request <https://renovatebot.com/docs/noise-reduction/#package-grouping>`_. This may
delay notification of security patch releases or make it harder to identify the
exact cause of a test failure, but also makes it less likely that the PRs will be
Expand Down
4 changes: 2 additions & 2 deletions oeps/processes/oep-0014-proc-archive-repos.rst
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ These steps should be followed for all repos within the Open edX organization (f
- Move the repository to the openedx-unsupported organization

- Remove references to the repository from the ``openedx-translations`` `repository <https://github.com/openedx/openedx-translations/>`_

- The entry in the ``extract-translation-source-files`` `workflow <https://github.com/openedx/openedx-translations/blob/main/.github/workflows/extract-translation-source-files.yml>`_

- The subdirectory in the ``translations`` `directory <https://github.com/openedx/openedx-translations/tree/main/translations>`_

- Create an ``axim-engineering`` `request <https://github.com/openedx/axim-engineering/issues/new/choose>`_ to remove the repository resource from the ``openedx-translations`` project on Transifex
Expand Down
4 changes: 2 additions & 2 deletions oeps/processes/oep-0021-proc-deprecation.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
OEP-21: Deprecation and Removal
#################################
###############################

+-----------------+--------------------------------------------------------+
| OEP | :doc:`OEP-21 <oep-0021-proc-deprecation>` |
Expand Down Expand Up @@ -317,7 +317,7 @@ The coordinator is responsible for:

When you are ready to coordinate the ticket, post a comment on the
GitHub issue saying you're doing so, and mark yourself as the GitHub
issue's owner. This can be done even if you don't have write access to
issue's owner. This can be done even if you don't have write access to
the ticket by making a comment on the ticket that says ``assign me``.

.. note::
Expand Down
3 changes: 2 additions & 1 deletion oeps/processes/oep-0054-core-contributors.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
OEP-54: Core Contributors
###########################
#########################

.. list-table::
:widths: 25 75

Expand Down
Loading