[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134878#3869947 , @mehdi_amini 
wrote:

> From what I saw when you posted the discourse thread initially, I understand 
> you're targeting user-visible features, and mostly from the "toolchain" side 
> of the project.
> However I find the language here to be potentially confusing for the API 
> surface of the libraries: that is how much of this applies to the LLVM C++ 
> libraries API surface?
> If this is out-of-scope, can this be called out more explicitly?

Sure! Would it help to add a paragraph before the bulleted list in `breaking` 
that says something along the lines of:

The C++ interfaces of individual library components of projects like LLVM or 
Clang are not intended to be stable interfaces. Potentially disruptive changes 
to such C++ interfaces do not constitute a breaking change unless the 
disruption is exposed via another mechanism such as a stable C API.

(Feel free to wordsmith it!)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

From what I saw when you posted the discourse thread initially, I understand 
you're targeting user-visible features, and mostly from the "toolchain" side of 
the project.
However I find the language here to be potentially confusing for the API 
surface of the libraries: that is how much of this applies to the LLVM C++ 
libraries API surface?
If this is out-of-scope, can this be called out more explicitly?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8673444598be: Update developer policy on potentially 
breaking changes (authored by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's 
code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading 
a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is to give vendors early notice that potentially disruptive changes
+  are being considered but have not yet been accepted. Vendors can give early
+  testing feedback on the changes to alert us to unacceptable breakages. The
+  current list of vendor groups is:
+
+  * `Clang vendors `_
+  * `libc++ vendors `_
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.
+
+* When committing the change to the repository, add appropriate information
+  about the potentially breaking changes to the ``Potentially Breaking 
Changes``
+  section of the project's release notes. The release note should have
+  information about what the change is, what is potentially disruptive about
+  it, as well as any code examples, links, and motivation that is appropriate
+  to share with users. This helps users to learn about potential issues with
+  upgrading to that release.
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the
+  `Announcements `_ channel on
+  Discourse. The post should be tagged with the ``potentially-breaking`` label
+  and a label specific to the project (such as ``clang``, ``llvm``, etc). This
+  is another mechanism by which we can give pre-release notice to users about
+  potentially disruptive changes. It is a lower-traffic alternative to the
+  joining "vendors" group. To automatically be notified of new announcements
+  with the ``potentially-breaking`` label, go to your user preferences page in
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``.
+
 .. _code owners:
 
 Code Owners
@@ -181,7 +226,12 @@
   programming paradigms.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
-  a future release, such as removal of a deprecated feature.
+  a future release, such as removal of a deprecated feature. In this case, the
+  release note should be added to a ``Potentially Breaking Changes`` section of
+  the notes with sufficient information and examples to demonstrate the
+  potential disruption. Additionally, any new entries to this section should be
+  announced in the `Announcements `_
+  channel on Discourse. See :ref:`breaking` for more details.
 
 Code reviewers are encouraged to request a release note if they think one is
 warranted when performing a code review.


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" 

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134878#3836840 , @aaron.ballman 
wrote:

> Pinging reviewers from projects other than libcxx (I'm hoping to get buy-in 
> from someone on the LLVM side of things; lldb would be nice as well).

I'll be landing this later today on the assumption that folks are comfortable 
with the changes here, so if you have concerns, bringing them up now would be 
appreciated.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Pinging reviewers from projects other than libcxx (I'm hoping to get buy-in 
from someone on the LLVM side of things; lldb would be nice as well).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

aaron.ballman wrote:
> ldionne wrote:
> > I wonder whether `Announcements` is truly a lower-traffic alternative to 
> > `vendors` groups, if we end up posting each potentially breaking change to 
> > the list and tagging vendors on each such review. I'm not against posting 
> > on Discourse, however it seems to me like basically another equivalent 
> > channel of communication for these changes (which might be beneficial, I'm 
> > neutral on that).
> The reason we have a split like that is for timing and chattiness. If you're 
> a downstream like Intel has with ICX, you might want to be in `clang-vendors` 
> so that you are involved in conversations about potentially breaking changes. 
> You'll be getting emails for all review comments on that review. But if 
> you're a downstream like a Gentoo package maintainer, you might not want to 
> be *that* involved in the development of the compiler, but still want to know 
> when changes are coming down the pipeline to do early pre-release testing 
> while there's still time to put the brakes on before a release goes out.
Okay, makes sense. Let's go for it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:129
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.

aaron.ballman wrote:
> Mordante wrote:
> > 
> I didn't apply this one because I think it's grammatically correct without 
> the comma.
I looks slightly odd to me, but I'm not a native speaker.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:129
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.

Mordante wrote:
> 
I didn't apply this one because I think it's grammatically correct without the 
comma.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 463949.
aaron.ballman marked 3 inline comments as done.
aaron.ballman added a comment.

Applying review feedback.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's 
code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading 
a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is to give vendors early notice that potentially disruptive changes
+  are being considered but have not yet been accepted. Vendors can give early
+  testing feedback on the changes to alert us to unacceptable breakages. The
+  current list of vendor groups is:
+
+  * `Clang vendors `_
+  * `libc++ vendors `_
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.
+
+* When committing the change to the repository, add appropriate information
+  about the potentially breaking changes to the ``Potentially Breaking 
Changes``
+  section of the project's release notes. The release note should have
+  information about what the change is, what is potentially disruptive about
+  it, as well as any code examples, links, and motivation that is appropriate
+  to share with users. This helps users to learn about potential issues with
+  upgrading to that release.
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the
+  `Announcements `_ channel on
+  Discourse. The post should be tagged with the ``potentially-breaking`` label
+  and a label specific to the project (such as ``clang``, ``llvm``, etc). This
+  is another mechanism by which we can give pre-release notice to users about
+  potentially disruptive changes. It is a lower-traffic alternative to the
+  joining "vendors" group. To automatically be notified of new announcements
+  with the ``potentially-breaking`` label, go to your user preferences page in
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``.
+
 .. _code owners:
 
 Code Owners
@@ -181,7 +226,12 @@
   programming paradigms.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
-  a future release, such as removal of a deprecated feature.
+  a future release, such as removal of a deprecated feature. In this case, the
+  release note should be added to a ``Potentially Breaking Changes`` section of
+  the notes with sufficient information and examples to demonstrate the
+  potential disruption. Additionally, any new entries to this section should be
+  announced in the `Announcements `_
+  channel on Discourse. See :ref:`breaking` for more details.
 
 Code reviewers are encouraged to request a release note if they think one is
 warranted when performing a code review.


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users and vendors of potential disruptions when upgrading to
+a newer version of a tool. For example, deprecating a feature that is expected
+to be removed in the future, removing an already-deprecated feature, upgrading a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is 

[PATCH] D134878: Update developer policy on potentially breaking changes

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.
This revision is now accepted and ready to land.

Thanks a lot for working on this! A few small nits, otherwise LGTM.




Comment at: llvm/docs/DeveloperPolicy.rst:112
+
+Please help notify users of potential disruptions when upgrading to a newer
+version of a tool. For example, deprecating a feature that is expected to be





Comment at: llvm/docs/DeveloperPolicy.rst:129
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.





Comment at: llvm/docs/DeveloperPolicy.rst:150
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``
+




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

ldionne wrote:
> I wonder whether `Announcements` is truly a lower-traffic alternative to 
> `vendors` groups, if we end up posting each potentially breaking change to 
> the list and tagging vendors on each such review. I'm not against posting on 
> Discourse, however it seems to me like basically another equivalent channel 
> of communication for these changes (which might be beneficial, I'm neutral on 
> that).
The reason we have a split like that is for timing and chattiness. If you're a 
downstream like Intel has with ICX, you might want to be in `clang-vendors` so 
that you are involved in conversations about potentially breaking changes. 
You'll be getting emails for all review comments on that review. But if you're 
a downstream like a Gentoo package maintainer, you might not want to be *that* 
involved in the development of the compiler, but still want to know when 
changes are coming down the pipeline to do early pre-release testing while 
there's still time to put the brakes on before a release goes out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

Thanks for working on this! FWIW, this more or less standardizes what we've 
been doing in libc++ for the past ~1.5 years and it's been pretty low effort 
for us to do. And putting my vendor hat on, it's been extremely useful for me 
to track down potential issues when trying to ship a new version of libc++. 
This LGTM, although I'm somewhat neutral on whether to post on Discourse as 
well as having the vendor groups on Phabricator. I'm not sure I understand the 
benefit of doing both, but I will happily conform if folks see value in it.




Comment at: llvm/docs/DeveloperPolicy.rst:140
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the

I wonder whether `Announcements` is truly a lower-traffic alternative to 
`vendors` groups, if we end up posting each potentially breaking change to the 
list and tagging vendors on each such review. I'm not against posting on 
Discourse, however it seems to me like basically another equivalent channel of 
communication for these changes (which might be beneficial, I'm neutral on 
that).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: JDevlieghere.
aaron.ballman added a comment.

Adding someone from lldb as a review for awareness.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: mehdi_amini.
aaron.ballman added a comment.

Note: I would love to add a vendors group to Phabricator for LLVM and lldb (and 
any other project we think has vendors), but this requires admin privileges to 
do. @mehdi_amini, is this something you could help out with assuming folks are 
in favor of the developer policy changes?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134878/new/

https://reviews.llvm.org/D134878

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134878: Update developer policy on potentially breaking changes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rnk, lattner, ldionne, rjmccall, Mordante, 
tahonermann, bruno, tonic.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: LLVM.

We've recently had issues appropriately notifying users and stakeholders of 
changes to projects that may be potentially disruptive when upgrading. This led 
to discussion on Discourse about how to improve the situation, which can be 
found at: 
https://discourse.llvm.org/t/rfc-add-new-discourse-channel-for-potentially-breaking-disruptive-changes-for-clang/65251

Ultimately, it sounds like we want to encourage three things:

1. Alert vendors during the code review so they can provide early feedback on 
potentially breaking changes that would be unacceptable for them.
2. Alert vendors and users after committing the changes so they can perform 
pre-release testing on a completed change to determine if it causes 
unreasonable problems for them.
3. Alert users more clearly through the release notes so that it's easier to 
determine how disruptive an upgrade might be.

This updates the developer policy accordingly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134878

Files:
  llvm/docs/DeveloperPolicy.rst


Index: llvm/docs/DeveloperPolicy.rst
===
--- llvm/docs/DeveloperPolicy.rst
+++ llvm/docs/DeveloperPolicy.rst
@@ -104,6 +104,51 @@
 software. Please see :doc:`CodeReview` for more information on LLVM's 
code-review
 process.
 
+.. _breaking:
+
+Making Potentially Breaking Changes
+---
+
+Please help notify users of potential disruptions when upgrading to a newer
+version of a tool. For example, deprecating a feature that is expected to be
+removed in the future, removing an already-deprecated feature, upgrading a
+diagnostic from a warning to an error, switching important default behavior, or
+any other potentially disruptive situation thought to be worth raising
+awareness of. For such changes, the following should be done:
+
+* When performing the code review for the change, please add any applicable
+  "vendors" group to the review for their awareness. The purpose of these
+  groups is to give vendors early notice that potentially disruptive changes
+  are being considered but have not yet been accepted. Vendors can give early
+  testing feedback on the changes to alert us to unacceptable breakages. The
+  current list of vendor groups is:
+
+  * `Clang vendors `_
+  * `libc++ vendors `_
+
+  People interested in joining the vendors group can do so by clicking the
+  "Join Project" link on the vendor's "Members" page in Phabricator.
+
+* When committing the change to the repository, add appropriate information
+  about the potentially breaking changes to the ``Potentially Breaking 
Changes``
+  section of the project's release notes. The release note should have
+  information about what the change is, what is potentially disruptive about
+  it, as well as any code examples, links, and motivation that is appropriate
+  to share with users. This helps users to learn about potential issues with
+  upgrading to that release.
+
+* After the change has been committed to the repository, the potentially
+  disruptive changes described in the release notes should be posted to the
+  `Announcements `_ channel on
+  Discourse. The post should be tagged with the ``potentially-breaking`` label
+  and a label specific to the project (such as ``clang``, ``llvm``, etc). This
+  is another mechanism by which we can give pre-release notice to users about
+  potentially disruptive changes. It is a lower-traffic alternative to the
+  joining "vendors" group. To automatically be notified of new announcements
+  with the ``potentially-breaking`` label, go to your user preferences page in
+  Discourse, and add the label to one of the watch categories under
+  ``Notifications->Tags``
+
 .. _code owners:
 
 Code Owners
@@ -181,7 +226,12 @@
   programming paradigms.
 * Modifying a C stable API.
 * Notifying users about a potentially disruptive change expected to be made in
-  a future release, such as removal of a deprecated feature.
+  a future release, such as removal of a deprecated feature. In this case, the
+  release note should be added to a ``Potentially Breaking Changes`` section of
+  the notes with sufficient information and examples to demonstrate the
+  potential disruption. Additionally, any new entries to this section should be
+  announced in the `Announcements `_
+  channel on Discourse. See :ref:`breaking` for more details.
 
 Code reviewers are encouraged to request a release note if they think one is
 warranted when performing a code review.


Index: llvm/docs/DeveloperPolicy.rst