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.

Sign up for GitHub

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

Docs/unify contrib docs #9742

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Docs/unify contrib docs #9742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f22b46
git clone command. graphviz note
Nov 9, 2024
e8f3790
unify contribution docs
Nov 9, 2024
7dbfeca
add Tambe T to contributors.txt
Nov 9, 2024
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
46 changes: 33 additions & 13 deletions CONTRIBUTING.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,56 @@
Contributing
============
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this title is lost entirely. Why?


Instructions for contributors
-----------------------------

In order to make a clone of the `GitHub <https://github.com/aio-libs/aiohttp>`_ repo: open the link and press the "Fork" button on the upper-right menu of the web page.

In order to make a clone of the GitHub_ repo: open the link and press the
"Fork" button on the upper-right menu of the web page.

I hope everybody knows how to work with git and github nowadays :)
If you'd like to learn more about Git and GitHub, `check out GitHub's helpful introduction
<https://docs.github.com/en/get-started/using-git/about-git>`_.

Workflow is pretty straightforward:

1. Clone the GitHub_ repo using the ``--recurse-submodules`` argument
0. Make sure you are reading the latest version of this document.
It can be found in the GitHub_ repo in the ``docs`` subdirectory.

1. Clone your forked GitHub_ repo with the ``--recurse-submodules`` flag as shown in the command below,
ensuring to replace the placeholder with your github username:

.. code-block:: shell

$ git clone \
https://github.com/<your_github_username>/aiohttp.git \
--recurse-submodules


2. Setup your machine with the required development environment

3. Make a change

4. Make sure all tests passed

5. Add a file into the ``CHANGES`` folder, named after the ticket or PR number
5. Add a file into the ``CHANGES`` folder (see `Changelog update <CHANGES>`_ for how).

Copy link
Member

Choose a reason for hiding this comment

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

This relative file reference only works on GitHub and is a broken link in Sphinx: https://aiohttp--9742.org.readthedocs.build/en/9742/contributing.html#instructions-for-contributors

6. Commit changes to your own aiohttp clone

7. Make a pull request from the github page of your clone against the master branch

8. Optionally make backport Pull Request(s) for landing a bug fix into released aiohttp versions.

.. important::
.. note::

The project uses *Squash-and-Merge* strategy for *GitHub Merge* button.

Basically it means that there is **no need to rebase** a Pull Request against
*master* branch. Just ``git merge`` *master* into your working copy (a fork) if
needed. The Pull Request is automatically squashed into the single commit
once the PR is accepted.

.. note::

GitHub issue and pull request threads are automatically locked when there has
not been any recent activity for one year. Please open a `new issue
<https://github.com/aio-libs/aiohttp/issues/new>`_ for related bugs.
Comment on lines +49 to +50
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is true anymore (I see locked issues from years ago, but don't think there's any auto-locking in the past 4 years). Maybe just reword to open a new issue instead of commenting on a closed issue.

Copy link
Member

Choose a reason for hiding this comment

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

Or we should re-establish the automation. It used to be run by https://github.com/apps/stale, but the app has been deprecated in favor of the action: https://github.com/marketplace/actions/close-stale-issues. We just never remembered to migrate.

Copy link
Member

Choose a reason for hiding this comment

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

Personally, I find auto-locked issues a little annoying. Though if it's after a year then I don't think it makes a difference, we don't tend to see anything on issues that have been closed that long. For this PR, I think it just needs to document the current status.


Please open the "`contributing <https://docs.aiohttp.org/en/stable/contributing.html>`_"
documentation page to get detailed information about all steps.
If you feel like there are important points in the locked discussions,
please include those excerpts into that new issue.

.. _GitHub: https://github.com/aio-libs/aiohttp
.. export-point-instructions-for-contributors
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ Sunit Deshpande
Sviatoslav Bulbakha
Sviatoslav Sydorenko
Taha Jahangir
Tambe Tabitha Achere
Taras Voinarovskyi
Terence Honles
Thanos Lefteris
Expand Down
66 changes: 17 additions & 49 deletions docs/contributing.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,61 +1,16 @@
.. _aiohttp-contributing:

Contributing
============
.. include:: ../CONTRIBUTING.rst
:end-before: export-point-instructions-for-contributors

(:doc:`contributing-admins`)
Copy link
Member

Choose a reason for hiding this comment

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

Why not keep this in the same place: at the top of the document?

https://aiohttp--9742.org.readthedocs.build/en/9742/contributing.html#instructions-for-contributors


Instructions for contributors
-----------------------------

In order to make a clone of the GitHub_ repo: open the link and press the "Fork" button on the upper-right menu of the web page.

I hope everybody knows how to work with git and github nowadays :)

Workflow is pretty straightforward:

0. Make sure you are reading the latest version of this document.
It can be found in the GitHub_ repo in the ``docs`` subdirectory.

1. Clone the GitHub_ repo using the ``--recurse-submodules`` argument

2. Setup your machine with the required development environment

3. Make a change

4. Make sure all tests passed

5. Add a file into the ``CHANGES`` folder (see `Changelog update`_ for how).

6. Commit changes to your own aiohttp clone

7. Make a pull request from the github page of your clone against the master branch

8. Optionally make backport Pull Request(s) for landing a bug fix into released aiohttp versions.

.. note::

The project uses *Squash-and-Merge* strategy for *GitHub Merge* button.

Basically it means that there is **no need to rebase** a Pull Request against
*master* branch. Just ``git merge`` *master* into your working copy (a fork) if
needed. The Pull Request is automatically squashed into the single commit
once the PR is accepted.

.. note::

GitHub issue and pull request threads are automatically locked when there has
not been any recent activity for one year. Please open a `new issue
<https://github.com/aio-libs/aiohttp/issues/new>`_ for related bugs.

If you feel like there are important points in the locked discussions,
please include those excerpts into that new issue.


Preconditions for running aiohttp test suite
--------------------------------------------

We expect you to use a python virtual environment to run our tests.
Also, ensure that you have `npm
<https://docs.npmjs.com/downloading-and-installing-node-js-and-npm>`_ installed.
Comment on lines +12 to +13
Copy link
Member

@Dreamsorcerer Dreamsorcerer Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is only for building llhttp, right? So, maybe:

Suggested change
Also, ensure that you have `npm
<https://docs.npmjs.com/downloading-and-installing-node-js-and-npm>`_ installed.
If you intend to run the parser tests, ensure that you have `npm
<https://docs.npmjs.com/downloading-and-installing-node-js-and-npm>`_ installed.


There are several ways to make a virtual environment.

Expand Down Expand Up @@ -237,10 +192,23 @@ Please before making a Pull Request about documentation changes run:
$ make doc

Once it finishes it will output the index html page

``open file:///.../aiohttp/docs/_build/html/index.html``.

Go to the link and make sure your doc changes looks good.

Note that this page doesn't automatically refresh itself. So, if you make more changes, build the docs again to see how they look on the page.

.. note::
If you are on MacOS, you might need to install `graphviz <https://graphviz.org/>`_ first.
Copy link
Member

@Dreamsorcerer Dreamsorcerer Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This isn't limited to MacOS, the PR added an apt package to make this work in the CI:
https://github.com/aio-libs/aiohttp/pull/9359/files#diff-cde814ef2f549dc093f5b8fc533b7e8f47e7b32a8081e0760e57d5c25a1139d9R17-R18

So, maybe something like:

You'll also need to install graphviz with your package manager (e.g. `apt install graphviz` or `brew install graphviz`).

Copy link
Member

Choose a reason for hiding this comment

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

As this is prerequisite, I'd move this step above the make doc command, so users see it first.

Use

.. code-block:: shell

$ brew install graphviz
Comment on lines +206 to +208
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this needs extra indentation.

Suggested change
.. code-block:: shell
$ brew install graphviz
.. code-block:: shell
$ brew install graphviz


and then run the command above to build the docs.

Spell checking
--------------

Expand Down
Loading