[llvm][docs] Extend docs on GitHub's "squash and merge"

[banach-space]

From what I’ve observed, some contributors are still unaware that in LLVM, the
PR summary - not the commit messages - is used as the final commit message when
merging. This is especially unclear to those without commit access, as only
users with commit access can edit the commit message before merging.

This PR clarifies that policy and consolidates all relevant information into
GitHub.rst, ensuring it is no longer split between GitHub.rst and
Contributing.rst.

Note, a big part of this change is merely moving text between the docs.

[DavidSpickett]

Same energy as https://llvm.org/docs/Contributing.html#how-to-submit-a-patch.

Repeating this point is ok, but do have a look and see what the overlap is right now. If we can reduce it maybe by saying "now to contribute via Github see here" then that would be great.

I have a feeling these pages will eventually merge, or the general contributing will delegate any code contribution parts to the GitHub page.

[banach-space]

Thanks for the review @DavidSpickett !

Same energy as https://llvm.org/docs/Contributing.html#how-to-submit-a-patch.

Identical energy. Great minds :)

My view is that https://llvm.org/docs/Contributing.html should only contain very high level info, e.g.:

• we use GitHub, go to that page for details
• to report a bug, go there
• to do that other important thing, follow this link.

And then, https://llvm.org/docs/GitHub.html should contain all the details on using GitHub.

Repeating this point is ok, but do have a look and see what the overlap is right now. If we can reduce it maybe by saying "now to contribute via Github see here" then that would be great.

Let me move a few things around. Let me know what you think.

[DavidSpickett]

I think the way I would go about finding what to link and what to duplicate here is to sit down with one of the pages and very literally follow it, adding anything that's missing. To end up with one document, that although long, contains everything a new contributor will need for the first PR. Then I could assess what the scale of changes would be if we were to change these 2 documents.

Which is way more work than what you wanted to do here, so don't do that for this.

With some shuffling these changes are ok but leave it at that so you don't end up in the weeds.

[banach-space]

I think the way I would go about finding what to link and what to duplicate here is to sit down with one of the pages and very literally follow it, adding anything that's missing. To end up with one document, that although long, contains everything a new contributor will need for the first PR. Then I could assess what the scale of changes would be if we were to change these 2 documents.

Agreed. But I think that we can simplify this task by basically removing all GitHub-related contend from Contributing.rst, i.e.

llvm-project/llvm/docs/Contributing.rst

Lines 53 to 182 in cd3acd1

	How to Submit a Patch
	=====================
	Once you have a patch ready, it is time to submit it. The patch should:
	* include a small unit test
	* conform to the :doc:`CodingStandards`. You can use the `clang-format-diff.py`_ or `git-clang-format`_ tools to automatically format your patch properly.
	* not contain any unrelated changes
	* be an isolated change. Independent changes should be submitted as separate patches as this makes reviewing easier.
	* have a single commit, up-to-date with the upstream ``origin/main`` branch, and don't have merges.
	.. _format patches:
	Before sending a patch for review, please also try to ensure it is
	formatted properly. We use ``clang-format`` for this, which has git integration
	through the ``git-clang-format`` script. On some systems, it may already be
	installed (or be installable via your package manager). If so, you can simply
	run it -- the following command will format only the code changed in the most
	recent commit:
	.. code-block:: console
	% git clang-format HEAD~1
	.. note::
	For some patches, formatting them may add changes that obscure the intent of
	the patch. For example, adding to an enum that was not previously formatted
	may result in the entire enum being reformatted. This happens because not all
	of the LLVM Project conforms to LLVM's clang-format style at this time.
	If you think that this might be the case for your changes, or are unsure, we
	recommend that you add the formatting changes as a **separate commit** within
	the Pull Request.
	Reviewers may request that this formatting commit be made into a separate Pull
	Request that will be merged before your actual changes.
	This means that if the formatting changes are the first commit, you will have
	an easier time doing this. If they are not, that is ok too, but you will have
	to do a bit more work to separate it out.
	Note that ``git clang-format`` modifies the files, but does not commit them --
	you will likely want to run one of the following to add the changes to a commit:
	.. code-block:: console
	# To create a new commit.
	% git commit -a
	# To add to the most recent commit.
	% git commit --amend -a
	.. note::
	If you don't already have ``clang-format`` or ``git clang-format`` installed
	on your system, the ``clang-format`` binary will be built alongside clang, and
	the git integration can be run from
	``clang/tools/clang-format/git-clang-format``.
	The LLVM project has migrated to GitHub Pull Requests as its review process.
	For more information about the workflow of using GitHub Pull Requests see our
	:ref:`GitHub <github-reviews>` documentation. We still have an read-only
	`LLVM's Phabricator <https://reviews.llvm.org>`_ instance.
	To make sure the right people see your patch, please select suitable reviewers
	and add them to your patch when requesting a review.
	Suitable reviewers are the maintainers of the project you are modifying, and
	anyone else working in the area your patch touches. To find maintainers, look for
	the ``Maintainers.md`` or ``Maintainers.rst`` file in the root of the project's
	sub-directory. For example, LLVM's is ``llvm/Maintainers.md`` and Clang's is
	``clang/Maintainers.rst``.
	If you are a new contributor, you will not be able to select reviewers in such a
	way, in which case you can still get the attention of potential reviewers by CC'ing
	them in a comment -- just @name them.
	If you have received no comments on your patch for a week, you can request a
	review by 'ping'ing the GitHub PR with "Ping" in a comment. The common courtesy 'ping' rate
	is once a week. Please remember that you are asking for valuable time from
	other professional developers.
	After your PR is approved, ensure that:
	* The PR title and description describe the final changes. These will be used
	as the title and message of the final squashed commit. The titles and
	messages of commits in the PR will **not** be used.
	* You have set a valid email address in your GitHub account, see :ref:`github-email-address`.
	Now you can merge your PR. If you do not have the ability to merge the PR, ask your
	reviewers to merge it on your behalf. You must do this explicitly, as reviewers'
	default assumption is that you are able to merge your own PR.
	For more information on LLVM's code-review process, please see
	:doc:`CodeReview`.
	.. _commit_from_git:
	For developers to commit changes from Git
	-----------------------------------------
	Once a patch is reviewed, you can select the "Squash and merge" button in the
	GitHub web interface.
	When pushing directly from the command-line to the ``main`` branch, you will need
	to rebase your change. LLVM has a linear-history policy, which means
	that merge commits are not allowed and the ``main`` branch is configured to reject
	pushes that include merges.
	GitHub will display a message that looks like this:
	.. code-block:: console
	remote: Bypassed rule violations for refs/heads/main:
	remote:
	remote: - Required status check “buildkite/github-pull-requests” is expected.
	This can seem scary, but this is just an artifact of the GitHub setup: it is
	intended as a warning for people merging pull-requests with failing CI. We can't
	disable it for people pushing on the command-line.
	Please ask for help if you're having trouble with your particular git workflow.
	.. _git_pre_push_hook:
	Git pre-push hook
	^^^^^^^^^^^^^^^^^
	We include an optional pre-push hook that run some sanity checks on the revisions
	you are about to push and ask confirmation if you push multiple commits at once.
	You can set it up (on Unix systems) by running from the repository root:
	.. code-block:: console

IMHO, it's going to be much easier if we are strict about not distributing GitHub related guidelines across multiple documents. Just food for thought :)

Which is way more work than what you wanted to do here, so don't do that for this.

Yes :)

With some shuffling these changes are ok but leave it at that so you don't end up in the weeds.

👍🏻

[DavidSpickett]

This is good, but maybe the PR description needs to be updated since this is now more like moving the existing note into a place where people will actually see it.

[DavidSpickett]

Also CI is failing.

[banach-space]

@ofri-frishman , your recent PR prompted me to write this update. Could you take a look and tell us whether it makes things clearer? Thanks!

[banach-space]

I also added a few more reviewers, mostly folks who have recently touched the update files. Please let me know if there's anyone else that we should include here. Thank you :)

[ofri-frishman]

@ofri-frishman , your recent PR prompted me to write this update. Could you take a look and tell us whether it makes things clearer? Thanks!

Yeah, looks good to me. I should probably go over these documents some more to better familiarize myself with all the aspects of contributing. I added a small comment regarding what I encountered.

[AaronBallman]

LGTM, thanks for the cleanup!

[DavidSpickett]

Looks good to me, thanks for dealing with this, it will help many people.