[docs] Refresh Developer Policy text #136198

rnk · 2025-04-17T21:04:44Z

Clarify lots of existing practice. Expand on the "major change" section,
which is the closest thing we have on how to run an RFC.

Clarify lots of existing practice. Expand on the "major change" section, which is the closest thing we have on how to run an RFC.

llvm/docs/DeveloperPolicy.rst

ftynse · 2025-04-18T08:00:17Z

llvm/docs/DeveloperPolicy.rst

-#. Once you have created your patch, create a
-   :ref:`GitHub Pull Request <github-reviews>` for
-   it (or commit it directly if applicable).
+* Identify 2-3 individuals to review the patch. Look through the relevant


Do random GIthub users have permissions to add PR reviewers? If not, we should document the recommended way of attracting attention, e.g., by @-pinging the relevant people in a comment and explicitly stating you don't have permission to add reviewers (for one-off PRs) and/or asking for "triage" set of permissions.

Do random GIthub users have permissions to add PR reviewers?

In my experience, they do not have the ability to do this themselves unless they're a member of the organization. Though I wonder if that can be relaxed?

I added guidance to @-ping in the absence of permission changes. It's good enough for the time being.

ftynse · 2025-04-18T08:00:41Z

llvm/docs/DeveloperPolicy.rst

+
+* Make sure your patch is based on a recent commit from ``main``, rather than a
+  previous release branch. If you want to make changes to a release branch, land
+  a change in ``main`` first and ask a release manager to backport it.


Should we encourage developers to backport their patches themselves, when possible?

I think I should link the instructions for backporting and leave it at that. It looks like the current state is that we have special github issue comments that bots pick up and act on:
https://llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches

Are you planning on adding that link here?

Yes, done, sorry I missed that.

ftynse · 2025-04-18T08:03:53Z

llvm/docs/DeveloperPolicy.rst

+archived, and that notices of confidentiality or non-disclosure cannot be
+respected.
+
+We accept code contributions as :ref:`GitHub Pull Requests <github-reviews>`.


Some thing I don't have a strong opinion on, but witnessed: some PRs or Github issue discussions grow very long and sometimes contentious, but much fewer people engage with them. Maybe we want some sort of soft guideline on when to move such a discussion to the forums for broader community involvement.

I agree that's useful information, but I think I'd move that to the specific CodeReview docs, and I don't want to touch that file in this PR. This section is supposed to be an overview of the main communication spaces (Discourse, Discord, GitHub).

ftynse · 2025-04-18T08:09:19Z

llvm/docs/DeveloperPolicy.rst

-  by :doc:`bugpoint <Bugpoint>` or manually. It is unacceptable to place an
-  entire failing program into ``llvm/test`` as this creates a *time-to-test*
-  burden on all developers. Please keep them short.
+* Test cases are written in the relevant input language of the relevant


Maybe we want to highlight more prominently that, whenever possible, tests should be written against a tool such as optrather than as unit tests and use command-line tools such as FileCheck to verify the output for testing efficiency reasons.

I scrapped this paragraph, reordered it, and mentioned our usual preferred approaches.

ftynse · 2025-04-18T08:12:16Z

llvm/docs/DeveloperPolicy.rst

-set right" will leave the reviewer wondering about which bits, and why they
-weren't right, while "Correctly set overflow bits in TargetInfo" conveys almost
-all there is to the change.
+Commit messages should communicate briefly what the change does, and provide


IMO, the "why" part is the most important in the commit message. We can understand the "what" from the commit itself, although it is useful to summarize the "what" in the message for the reviewer to check whether the code actually does what the author intended.

ftynse · 2025-04-18T08:15:57Z

llvm/docs/DeveloperPolicy.rst

+messages should be thoughtfully written and specific, rather than vague. For
+example, "bits were not set right" will leave the reviewer wondering about which
+bits, and why they weren't right, while "Correctly set overflow bits in
+TargetInfo" conveys almost all there is to the change.


I'd also remind that the final commit, potentially squashed from multiple commits in a PR, will have the description taken from the PR description. The latter is set once when the PR is created, defaulting to the commit message if the PR only contains one commit, and does not track the changes to the commit message. As a corollary, developers must manually update the PR description as they iterate on the change...

I added a paragraph after the following bullets

ftynse · 2025-04-18T08:23:46Z

llvm/docs/DeveloperPolicy.rst

+often be helpful in making design discussions more concrete by demonstrating
+what is possible.
+
+TODO: Elaborate on best practices for making a successful RFC.


Random ideas for this from recent experience:

make it targeted, don't touch components irrelevant to the task

explain how the change improves LLVM for all/most stakeholders rather than your specific use case

periodically summarize the current state of the discussion and clearly separate points where consensus seems to emerge from those where further discussion is necessary

by default, the burden of proof is on the proposer (though we'd like to avoid situations when somebody keeps asking for more proof as a way of exhausting the proposer)

I added that list, but I'm sure we can add more here :) I think my goal with this PR was to get a better base for future edits.

AaronBallman · 2025-04-18T11:17:12Z

which is the closest thing we have on how to run an RFC.

FYI:
https://clang.llvm.org/get_involved.html#criteria, starting from The Clang community uses an RFC process

AaronBallman

Thank you for working on this refresh, it's definitely heading in a good direction IMO! I have some light feedback, still thinking about content further.

llvm/docs/DeveloperPolicy.rst

AaronBallman · 2025-04-18T11:20:24Z

llvm/docs/DeveloperPolicy.rst


 Making and Submitting a Patch
 -----------------------------

-When making a patch for review, the goal is to make it as easy for the reviewer
-to read it as possible.  As such, we recommend that you:
+Submitting patches for review is straightforward with GitHub. Follow the


Submitting patches for review is straightforward with GitHub.

I'd probably reword this to "Patches are submitted through GitHub" rather than claim it's straightforward for everyone (not everyone is familiar with GitHub or comfortable with git in general).

AaronBallman · 2025-04-18T11:21:15Z

llvm/docs/DeveloperPolicy.rst

-#. Once you have created your patch, create a
-   :ref:`GitHub Pull Request <github-reviews>` for
-   it (or commit it directly if applicable).
+* Identify 2-3 individuals to review the patch. Look through the relevant


Do random GIthub users have permissions to add PR reviewers?

In my experience, they do not have the ability to do this themselves unless they're a member of the organization. Though I wonder if that can be relaxed?

llvm/docs/DeveloperPolicy.rst

AaronBallman · 2025-04-18T11:24:45Z

llvm/docs/DeveloperPolicy.rst

+feature tests only. More extensive test cases (e.g., entire applications,
+benchmarks, etc) should be added to the ``llvm-test`` test suite.  The ``llvm-test``
+suite is for integration and application testing (correctness, performance, etc)
+testing, not feature or regression testing.


Do we also want to mention that it's the place to put tests which are incompatible with the license from the main LLVM repo?

Sure, done.

Co-authored-by: Oleksandr "Alex" Zinenko <[email protected]>

Co-authored-by: Aaron Ballman <[email protected]>

…-policy-refresh

rnk

Thanks for the suggestions, I think it's actually significantly better. Sorry for the long hiatus.

rnk · 2025-06-25T02:21:30Z

llvm/docs/DeveloperPolicy.rst

+
+* Make sure your patch is based on a recent commit from ``main``, rather than a
+  previous release branch. If you want to make changes to a release branch, land
+  a change in ``main`` first and ask a release manager to backport it.


I think I should link the instructions for backporting and leave it at that. It looks like the current state is that we have special github issue comments that bots pick up and act on:
https://llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches

rnk · 2025-06-25T02:30:19Z

llvm/docs/DeveloperPolicy.rst

+archived, and that notices of confidentiality or non-disclosure cannot be
+respected.
+
+We accept code contributions as :ref:`GitHub Pull Requests <github-reviews>`.


I agree that's useful information, but I think I'd move that to the specific CodeReview docs, and I don't want to touch that file in this PR. This section is supposed to be an overview of the main communication spaces (Discourse, Discord, GitHub).

rnk · 2025-06-25T02:32:45Z

llvm/docs/DeveloperPolicy.rst

-#. Once you have created your patch, create a
-   :ref:`GitHub Pull Request <github-reviews>` for
-   it (or commit it directly if applicable).
+* Identify 2-3 individuals to review the patch. Look through the relevant


I added guidance to @-ping in the absence of permission changes. It's good enough for the time being.

rnk · 2025-06-25T02:42:40Z

llvm/docs/DeveloperPolicy.rst

-  by :doc:`bugpoint <Bugpoint>` or manually. It is unacceptable to place an
-  entire failing program into ``llvm/test`` as this creates a *time-to-test*
-  burden on all developers. Please keep them short.
+* Test cases are written in the relevant input language of the relevant


I scrapped this paragraph, reordered it, and mentioned our usual preferred approaches.

rnk · 2025-06-25T02:45:53Z

llvm/docs/DeveloperPolicy.rst

+feature tests only. More extensive test cases (e.g., entire applications,
+benchmarks, etc) should be added to the ``llvm-test`` test suite.  The ``llvm-test``
+suite is for integration and application testing (correctness, performance, etc)
+testing, not feature or regression testing.


Sure, done.

rnk · 2025-06-25T02:53:52Z

llvm/docs/DeveloperPolicy.rst

+messages should be thoughtfully written and specific, rather than vague. For
+example, "bits were not set right" will leave the reviewer wondering about which
+bits, and why they weren't right, while "Correctly set overflow bits in
+TargetInfo" conveys almost all there is to the change.


I added a paragraph after the following bullets

rnk · 2025-06-25T03:01:02Z

llvm/docs/DeveloperPolicy.rst

+often be helpful in making design discussions more concrete by demonstrating
+what is possible.
+
+TODO: Elaborate on best practices for making a successful RFC.


I added that list, but I'm sure we can add more here :) I think my goal with this PR was to get a better base for future edits.

rnk · 2025-07-01T17:18:15Z

Ping, PTAL.

llvm/docs/DeveloperPolicy.rst

AaronBallman · 2025-07-01T18:07:59Z

llvm/docs/DeveloperPolicy.rst

+the firehose of all issue notifications, which some community members use to
+perform custom filtering.
+
+Beyond the main formal communication channels, LLVM has a Discord server for


This makes it sound like working groups are not formal communication channels. Perhaps:

Beyond the Discourse forums, LLVM has...

instead?

I said "asynchronous written", since I think that's the differentiator.

llvm/docs/DeveloperPolicy.rst

AaronBallman · 2025-07-01T18:11:29Z

llvm/docs/DeveloperPolicy.rst

+
+* Make sure your patch is based on a recent commit from ``main``, rather than a
+  previous release branch. If you want to make changes to a release branch, land
+  a change in ``main`` first and ask a release manager to backport it.


Are you planning on adding that link here?

llvm/docs/DeveloperPolicy.rst

Co-authored-by: Aaron Ballman <[email protected]>

AaronBallman

LGTM aside from a formatting nit. Thank you for working on the refresh!

llvm/docs/DeveloperPolicy.rst

Co-authored-by: Aaron Ballman <[email protected]>

…-policy-refresh

rnk · 2025-07-02T23:17:24Z

Thanks for the review!

I'm about to go on vacation for a while, and given that this is all doc text updates with no intended policy changes, I think I should go ahead and land them. I'm happy to address any unaddressed concerns from @ftynse in two weeks, and I think folks will benefit from the updates in the meantime.

[docs] Refresh Developer Policy text

01b92b7

Clarify lots of existing practice. Expand on the "major change" section, which is the closest thing we have on how to run an RFC.

rnk requested review from ftynse, AaronBallman and dwblaikie April 17, 2025 21:05

ftynse reviewed Apr 18, 2025

View reviewed changes

AaronBallman reviewed Apr 18, 2025

View reviewed changes

rnk and others added 5 commits June 24, 2025 20:15

Merge branch 'main' into dev-policy-refresh

838677b

fix grammar nit

3b37b82

Co-authored-by: Oleksandr "Alex" Zinenko <[email protected]>

Apply suggestions from code review

c7cf211

Co-authored-by: Aaron Ballman <[email protected]>

Merge remote-tracking branch 'rnk-github/dev-policy-refresh' into dev…

63dfba1

…-policy-refresh

add significant new text

fa96d72

rnk commented Jun 25, 2025

View reviewed changes

rnk marked this pull request as ready for review June 25, 2025 03:10

AaronBallman reviewed Jul 1, 2025

View reviewed changes

rnk and others added 2 commits July 1, 2025 12:58

Apply suggestions from code review

d0685cf

Co-authored-by: Aaron Ballman <[email protected]>

More docs from the review

2bdeb0d

AaronBallman approved these changes Jul 2, 2025

View reviewed changes

llvm/docs/DeveloperPolicy.rst Outdated Show resolved Hide resolved

rnk and others added 3 commits July 2, 2025 16:13

Merge branch 'main' into dev-policy-refresh

065840b

Update llvm/docs/DeveloperPolicy.rst

94f4902

Co-authored-by: Aaron Ballman <[email protected]>

Merge remote-tracking branch 'rnk-github/dev-policy-refresh' into dev…

1ba9e25

…-policy-refresh

rnk merged commit f01017c into llvm:main Jul 2, 2025
10 checks passed

[docs] Refresh Developer Policy text #136198

[docs] Refresh Developer Policy text #136198

Uh oh!

Conversation

rnk commented Apr 17, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AaronBallman commented Apr 18, 2025

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rnk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rnk commented Jul 1, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!