Tidy up contributing docs #4469

s-makin · 2023-09-28T18:51:05Z

Tidy up contributing docs

In preparation for Hacktoberfest, but also because this was needed,
made a first proper go at reorganising the contributor docs. 

- Organise docs by: Overview - Code - Docs - Community
- Split out some content into separate files to make it more obvious where to find the content
- Simplify new contributor docs
- Add docs_layout to mirror the dir_layout file
- Make docs style guide separate
- Move introduction to explanation and include front page link to help direct people to it

- Organise docs by: Overview - Code - Docs - Community - Split out some content into separate files to make it more obvious where to find the content - Simplify new contributor docs - Add docs_layout to mirror the dir_layout file - Make docs style guide separate - Move introduction to explanation and include front page link to help direct people to it

TheRealFalcon

I left some inline comments. Overall great content that'll be way more helpful to newcomers.

Is there a reason development/index.html no longer renders the CONTRIBUTING doc in Github? I think the separation causes a few issues:

If I'm coming into the docs via the Github CONTRIBUTING.[rst|md] (most people will), I saw the same headers/introduction and didn't immediately realize there was more content beneath the Github version. I think it's confusing having to find out where to pick up on the new page.
It means the two sources will likely diverge over time, and I'm not sure that's desirable.

Also, I just now realized that the section headings on the left sidebar are pages in and of themselves.

E.g., Contributing overview is a page unto itself and not just a a heading container for what's below it. That's usually not what I expect from dropdown menus and probably won't be obvious because clicking the arrows doesn't open the page. The headers in the non-development section are essentially a really simple (one or two sentence) overview with links to the rest of the pages, but this PR adds quite a lot of content to the new headers. Would it make more sense to include a separate Overview page at the top of the children, then add a link from the heading to that overview? That way, important information won't be hidden in these section headers.

TheRealFalcon · 2023-09-29T15:00:43Z

doc/rtd/development/index.rst


-   docs.rst
+For existing contributors


The first bullet under here is a first-time contributor activity and we'll want first time contributors to do everything listed. I'm nervous a first time contributor would be tempted to gloss over this section as they're not an 'existing' contributor. The subheadings could perhaps stand on their own.

TheRealFalcon · 2023-09-29T15:03:10Z

doc/rtd/development/index.rst

+* Add or update any :ref:`unit tests<testing>` accordingly.
+* Add or update any :ref:`integration_tests` (if applicable).
+* Format code (using ``black`` and ``isort``) with ``tox -e do_format``.
+* Run unit tests and/or linting checks pass using ``tox``.


Suggested change

* Run unit tests and/or linting checks pass using ``tox``.

* Ensure unit tests and/or linting checks pass using ``tox``.

or

Suggested change

* Run unit tests and/or linting checks pass using ``tox``.

* Run unit tests and/or linting checks using ``tox``.

TheRealFalcon · 2023-09-29T15:05:19Z

doc/rtd/development/index.rst


-   summit.rst
+Transferring CLA Signatures from Launchpad to GitHub


At this point, we have been on GitHub long enough that we can delete this section.

TheRealFalcon · 2023-09-29T15:09:48Z

doc/rtd/development/find_issues.rst

+You can express your interest in an issue by posting a comment on it. You
+should only work on one open issue at a time to avoid overloading yourself.
+
+If you have not submitted a PR (or a draft/"work-in-progress" PR) within a few


A week feels a bit more reasonable to me. I think we can allow a comment explaining their status too as that shows they are still engaged. The big thing we want to avoid is "I'm working on this" and then them ghosting.

TheRealFalcon · 2023-09-29T15:11:52Z

doc/rtd/development/find_issues.rst

+the PR commit message will link the issue to your proposed fix. This is the
+``Fixes GH-0000`` line in the template PR commit message.
+
+We will only assign an issue if there is a PR in progress.


Let's drop this line. If there's a linked PR, other people will already know not to work on it. Realistically, we'll probably only assign the PR in Github if it's a contributor we already have a history with.

TheRealFalcon · 2023-09-29T15:51:54Z

doc/rtd/development/code_review.rst

-   will perform an initial review, and monitor the PR to ensure that
-   the **Proposer** is receiving any assistance that they require. The
-   **Committers** will perform this assignment on a daily basis.
+The assigned committer can delegate the code review of a PR to another reviewer


I'm not sure this needs to be explicitly stated

TheRealFalcon · 2023-09-29T16:03:14Z

doc/rtd/development/dir_layout.rst

@@ -3,7 +3,7 @@
 Directory layout
 ****************

-``Cloud-init``'s directory structure is somewhat different from a regular
+Cloud-init's directory structure is somewhat different from a regular


We should remove the directory listing listed immediately after this line. It is woefully out of date.

TheRealFalcon · 2023-09-29T16:08:33Z

doc/rtd/development/style_docs.rst

-  capitalised at the start of a sentence (e.g., ``Cloud-init``).
- ``metadata``, ``datasource``: One word.
- ``user data``, ``vendor data``: Two words, not to be combined or hyphenated.
+- **Cloud-init**: Always hyphenated, and follows sentence case, so only


I would leave this lowercase. The point of the bullet is that we generally don't want the name of the project capitalized.

TheRealFalcon · 2023-09-29T16:13:29Z

doc/rtd/development/docs_layout.rst

+=============
+
+
+``man/``


Contains the Linux man pages for the binaries provided by cloud-init.

TheRealFalcon · 2023-09-29T16:16:34Z

doc/rtd/development/docs_layout.rst

+When the documentation is built locally using ``tox -e doc``, the built pages
+can be found in this folder.
+
+``sources/``


This is...uh...weird. It contains an example project of how to use some particular functionality (OVF) within cloud-init. I'm guessing the original intention was to keep such demos here, but I didn't know this existed and not really sure it belongs here. We should probably document it as is until we move it though.

TheRealFalcon · 2023-09-29T16:31:33Z

doc/rtd/development/first_PR.rst

+
+- Click :guilabel:`Pull Request`.
+
+- Fill out the pull request title, summarizing the change and a longer message


Hrm...I thought I had included a comment here earlier.

Since we have a PR template that is automatically populated anytime somebody creates a pull request, I'm not sure we need this template here too. The two have already diverged. We can instead say that when submitting a PR to fill out all pre-populated fields accordingly.

Yeah I did wonder about that, but since this looked more like an example I didn't want to just remove it. Happy to take it out though for ongoing maintenance reasons.

TheRealFalcon · 2023-10-02T22:07:42Z

I pushed a commit with my changes (not including the larger heading ones I mentioned).

@blackboxsw wanna review too? Since I already reviewed, we probably don't need a super deep review here.

TheRealFalcon · 2023-10-02T22:08:24Z

doc/rtd/development/first_PR.rst

@@ -0,0 +1,81 @@
+Submit your first pull request


I decided to remove quite a lot from this file and instead point the GitHub quickstart guide. Let me know if you think that was a mistake.

Original details were in the old contributing.rst.

blackboxsw · 2023-10-03T21:15:33Z

doc/rtd/development/dir_layout.rst

 ``/var/lib/cloud``
+==================


In dropping the top directory tree, we now have slightly confusing rendering of the subdirectories as headings equal to the font and weight of the parent /var/lib/cloud. Let's make them subheadings so they are deemphasized.

I've changed this instead to ellipsis to represent that the elements are subdirectories until /var/lib/cloud. The result looks like this:

blackboxsw

LGTM

James applied his review comments to this branch. So, that is an "approve" in my book

s-makin added 2 commits September 28, 2023 19:44

Fix conf.py issue

d47ff68

TheRealFalcon previously requested changes Sep 29, 2023

View reviewed changes

TheRealFalcon reviewed Sep 29, 2023

View reviewed changes

TheRealFalcon reviewed Oct 2, 2023

View reviewed changes

TheRealFalcon requested a review from blackboxsw October 2, 2023 22:08

blackboxsw reviewed Oct 3, 2023

View reviewed changes

apply my comments

03b3337

blackboxsw force-pushed the docs-contributing branch from bbb08b4 to 03b3337 Compare October 4, 2023 04:19

blackboxsw approved these changes Oct 4, 2023

View reviewed changes

blackboxsw merged commit 271ec8b into canonical:main Oct 4, 2023
26 checks passed

aciba90 mentioned this pull request Oct 16, 2023

Doc contrib links #4512

Merged

3 tasks

s-makin deleted the docs-contributing branch August 30, 2024 13:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tidy up contributing docs #4469

Tidy up contributing docs #4469

s-makin commented Sep 28, 2023

TheRealFalcon left a comment •

edited

Loading

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

TheRealFalcon Sep 29, 2023

s-makin Sep 29, 2023

TheRealFalcon commented Oct 2, 2023

TheRealFalcon Oct 2, 2023 •

edited

Loading

blackboxsw Oct 3, 2023

blackboxsw Oct 4, 2023

blackboxsw left a comment

	* Run unit tests and/or linting checks pass using ``tox``.
	* Ensure unit tests and/or linting checks pass using ``tox``.

	* Run unit tests and/or linting checks pass using ``tox``.
	* Run unit tests and/or linting checks using ``tox``.


		summit.rst
		Transferring CLA Signatures from Launchpad to GitHub


		- Click :guilabel:`Pull Request`.

		- Fill out the pull request title, summarizing the change and a longer message

		=============


		``man/``

		``/var/lib/cloud``
		==================

Tidy up contributing docs #4469

Tidy up contributing docs #4469

Conversation

s-makin commented Sep 28, 2023

TheRealFalcon left a comment • edited Loading

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

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

TheRealFalcon commented Oct 2, 2023

TheRealFalcon Oct 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blackboxsw left a comment

Choose a reason for hiding this comment

TheRealFalcon left a comment •

edited

Loading

TheRealFalcon Oct 2, 2023 •

edited

Loading