docs: eliminate unnecessary information from first contribution page

[holmanb]

Proposed Commit Message

docs: eliminate duplicate information from first PR page

First time contributors primarily need to understand what is expected in a
contribution. This creates a self-contained page that explains the bulk of
those expectations in a streamlined fashion that minimizes the amount of
additional unnecessary information. It also eliminates some redundancy,
and removes the unnecessarily verbose code review page.

Context

Expectations regarding contributions were distributed throughout various pages - in some cases intermixed with unrelated information.

The ideas communicated in the code review page could be communicated to contributors in far simpler terms.

This PR addresses both.

Changed files

new files

dev_docs.rst -> container for non-user-facing details

renamed files

packaging.rst -> build_system.rst
package_testing.rst -> build_distro_packaging_from_source.rst
first_PR.html -> development/index.html

split files

testing.rst -> howto/test_unreleased_packages.rst

deleted files

contribute_testing.rst

Test Steps

Merge type

• Squash merge using "Proposed Commit Message"
• Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

[TheRealFalcon]

Nice additions, but I'm -1 to most of the changes.

[TheRealFalcon]

I do wonder if it's worth putting a consolidated version of some of this info in the top-level README of the github project. Most projects have some sort of "getting started" section in their README that includes how to run the code and run tests. It could just be some kind of statement about how running the code requires some work with a link to the tutorial, and any other operations involve running tox.

[holmanb]

Thanks for the early feedback @TheRealFalcon. I addressed your comments, and also clarified the intent in the proposed commit message in the description.

[ottok]

The project has a Makefile, and it has a default target, test targets, packaging targets such as:

make
make fmt
make fmt-tip

They are not explained anywhere. As a contributor, I would expect that after cloing the project I can just run make and that the README in the project would also explain some basics. Now one needs to read an external website that has tox commands that are not the same as what the Makefile runs. In fact, I think I will probably close #6507 as it seems that there is no configuration where the Makefile targets work without breaking the CI, giving the impression that the developers in this project are actually not using the Makefile at all.

Also, on the topic of development practices, your style here to write the commit message in a comment and then have it manually added while squashing seems overly complex. Why don't you just rebase the PR contents to be one single commit with that commit message to beging with? I don't think there is any benefit in pushing separate WIP commits on the PR, you could just amend/rebase the existing one and then when ready a single Merge button press is enough, without additional commit message editing steps.

Third thing about contributions is that you should remove the label hacktoberfest from this project. It gives the impression that you are participating in the Hacktoberfest event, but in reality the project has never awarded any hacktoberfest-accepted labels to any PR submitted or merged in any October. You don't even have that label, but just the label hacktoberfest which is not following the Hacktoberfest convetion correctly (and has never been awarded to anyone).

[holmanb]

As a contributor, I would expect that after cloing the project I can just run make and that the README in the project would also explain some basics.

The README points to the contribution docs, which should have the information required to get started as a contributor. It isn't a valuable use of developer time to maintain duplicates of the same information. Make isn't a universal tool that is always used by developers. As someone who regularly contributes to various projects, I can tell you that every project has different contribution expectations, and the best way to jump into a project is to first read the contribution docs. This project is no different - intuition might work sometimes, but sometimes it won't.

[blackboxsw]

Thank you for the continued conversation here @ottok:

It gives the impression that you are participating in the Hacktoberfest event, but in reality the project has never awarded any hacktoberfest-accepted labels to any PR submitted or merged in any October. You don't even have that label, but just the label hacktoberfest which is not following the Hacktoberfest convetion correctly (and has never been awarded to anyone).

I think either you or I have misread or misinterpreted the Hacktoberfest rules for participating projects and we'll happily oblige if there is a demonstable failure here to properly categorize or award I don't think hacktoberfest-accepted label is required for awards or participation. I read the following quote for hacktoberfest participation to mean that a repo need only subscribe to hacktoberfest topic and PRs need to be merged during October with the related label hacktoberfest on the issue:
-- [participating] Your PR/MRs must be in a repo tagged with the “hacktoberfest” topic, or have the “hacktoberfest-accepted” label.
-- [accepted] Your PR/MRs must be merged, have the “hacktoberfest-accepted” label, or have an overall approving review.

If we think that this still doesn't meet the requirements for proper awarding for hacktoberfest contributions, please site the documentation that states clearly otherwise and we'll make sure to categorize such contributions as we ❤️ Hacktoberfest contributors.

[blackboxsw]

1. We've redacted this index to something that only drives contributors to code contribution via first-PR, and omits bug filing, docs or testing contributions.

Can we retain cross-links to the other contribution scopes otherwise this index page feels spartan and leaves the other nested menu items form the left-side navigation without much context.

• Contribute code
• Contribute to documentation
• Contribute testing
• Contribute bugs or feature requests

2. Also, let's fix the typo resulting in bogus sphinx anchor

Suggested change

	Make sure to read the `first PR guide<first-PR>` before starting
	Make sure to read the `first PR guide<first_PR>` before starting

The left-side menu has now become hard to navigate, the index.html now showcases two direct links to "Contribution requirements" and "Find issues to work on" which appear to live outside the subtopic "Contribute to code" number of top-level links which should live under "Contribute to code.

When compared with existing upstream left-side menu docs below, it feels a bit disorganized.

[blackboxsw]

Thank you brett. This looks great. Minor nits as you are wrapping things up here.

[holmanb]

@blackboxsw Thanks for the review. This grew in scope a bit as I redid the hierarchy to make contribution docs more streamlined - it became apparent that some things categorized under "development" were better categorized differently, and some pages had content which was out of place. I included some brief explanations for specific changes.

A couple of specific things to note:

Anywhere that "file a bug" is requested should now point to the howto file a bug page rather than the github tracker - the exception to that being of course the howto page itself.

The development directory has a clearer flow for readers: rather than 6 different pages to choose from as different possible entry points to "development", there is now a single first landing page which more clearly states the expectations on contributions, and beneath the top landing page there are three options to choose from in the toctree.

This changes the structure a bit, so we will want a couple of redirects.

[holmanb]

These are lower-frequency user needs, move them lower in the list

[holmanb]

This is developer documentation. Hide these pages from user distraction under a deeper taxonomy - and encourage developers to first read index.html.

[holmanb]

Direct bug reporters to the "how to file a bug guide" instead.

[holmanb]

This is documented on index.rst and README. It doesn't need to be duplicated here too.

[blackboxsw]

I like your new categorization of dev_docs to provide more structure around developer needs. It clarifies the messaging a bit and the intent of this area of our documentation.

Minor suggestions throughout the only icky things are:

1. the duplicates issues link in doc/rtd/howto/bugs.rst. Feel free to resolve that either local to howto/bugs.rst or more broadly via doc/rtd/links.txt
2. broken link to github with leading underscore

I think we are missing another doc file rename in this changeset. We need to be a bit careful about those renames as they generate 404s on RTD.com/docs.cloud-init.io etc that require admin account redirects for our project.

To ensure we better track file removals and renames, let's make sure we represent all doc filename changes in the PR description as you have

What's missing:

• first_PR.html -> development/index.html maybe?

After this lands I'll update rtd.com/cloud-init admin settings to ensure the proper redirects are added just in case external sites are linking some of these docs.

[blackboxsw]

I realized it's common convention, but shall we define pull request (PR) here at the top given this is the landing page for slightly inexperienced developers?
Minor suggestion s/that/which and s/unlikely to be merged/unlikely to merge/

Suggested change

	The following steps must be completed to develop cloud-init. A PR that
	fails to meet the contribution requirements is unlikely to be merged.
	The following steps must be completed to develop cloud-init. A pull request
	(PR) which fails to meet the contribution requirements is unlikely to merge.

[holmanb]

I realized it's common convention, but shall we define pull request (PR) here at the top

I don't think that we need to define standard industry terms that new contributors can find elsewhere. I don't this we need to tell the majority of contributors what a PR is. Any new contributor that wants to modify the project but isn't familiar with pull requests should be willing and able to figure out what that means - learning what you don't already know is part of development.

That said, I'm fine if we would rather replace "PR" with "pull request" in this instance - on the expectation that after getting past the github intro they aught to have figured it out.

[blackboxsw]

Shall we document our 'newer' incomplete tag policy?
An **incomplete** tag is manually placed on a PR by a maintainer seeking feedback or changes from the author. It will not be reviewed without an updated response or commit from the author.

[holmanb]

That seems redundant with what is already there:

Changes requested by core developers must be resolved before the PR can merge.

Which communicates that expectation.

I don't think the tags really represent a strong policy, rather they make it easier for devs to track the current state of a PR. It makes it slightly simpler to check if a PR has seen updates since a point in time, but the tag itself should be redundant with a developer request for changes.

[blackboxsw]

We are referencing https://github.com/canonical/cloud-init/issues in a lot of pages and we have duplication with "open issue`" and "Report an upstream cloud-init bug" in doc/rtd/howto/bugs.rst.

Might it make sense to include upstream_issues in doc/rtd/links.txt and including that file using .. include:: ../links.txt in all the places we link to upstream issues?

[holmanb]

We are referencing https://github.com/canonical/cloud-init/issues in a lot of pages

We're not, actually - I count two pages. This change intentionally replaced all pointers to https://github.com/canonical/cloud-init/issues with references to howto/bugs.rst. The intention there was to help users understand what makes a useful bug report as a step in the process of directing them to make a bug report. The only exceptions to that rule are howto/bugs.rst, and find_bugs.rst (which is relatively low-value since it documents more fairly standard industry practice) which is not intended for reporting bugs.

[holmanb]

Thanks for the review @blackboxsw. I fixed the broken link, eliminated the duplicate in bugs.rst, and updated the description.

I'll merge this and we can iterate as needed.