Add initial pass of deprecation guide post RFC #141

hultonha · 2022-07-27T10:57:50Z

This is a follow-on from the O3DE Deprecation Strategy RFC.

This is a first pass at creating a deprecation document engineers can use as a guide. It is not perfect nor exhaustive, but can be something we continue to refine and improve over time.

Signed-off-by: Tom Hulton-Harrop [email protected]

Signed-off-by: Tom Hulton-Harrop <[email protected]>

willihay

As a writer, the near 100% omission of sentence-completing periods (".") raises an eyebrow 😉, but otherwise this is clearly written. I just have a few comments.

guides/deprecation.md

willihay · 2022-07-27T15:58:37Z

guides/deprecation.md

+## Philosophy
+
+- When modifying existing code that external customers rely on, we should strive wherever possible to not break them when making changes
+- If code is to be changed or removed, we should let customers know ahead of time with a reasonable notice period (approximately 1 release (2-3 months))


Is "2-3 months" accurate going forward?

Hmm yeah might be more like 3-4 given when the last release happened 🤔 Or just omit and say 1 release

I prefer the latter. No need to reference time when release sequence is more important here.

Release will start accelerating as we become better at making them and as feature stabilize (requiring less testing churn). However, a concern with saying 1 release is that what if someone deprecates a feature or API call right before the next release. Maybe we can reference 1 full release cycle?

guides/deprecation.md

hultonha · 2022-07-27T16:43:27Z

As a writer, the near 100% omission of sentence-completing periods (".") raise an eyebrow 😉, but otherwise this is clearly written. I just have a few comments.

😅 sorry, I just picked a convention and tried to be consistent, I can go through and add them all (I just didn't want a mix of having some on some lines any not on others). Thanks for the feedback, will take a look back tomorrow 👍 Cheers!

Add apostrophe Co-authored-by: William Hayward <[email protected]> Signed-off-by: Tom Hulton-Harrop <[email protected]>

cgalvan · 2022-07-27T17:01:32Z

guides/deprecation.md

+    ```
+
+    - Add an announcement to the Discord Impactful Change channel
+      - Note: If you do not have permissions for this, please contact the sig your deprecation applies to and someone will post the impactful change announcement on your behalf


Here and in the next section the guide mentions locating the responsible sig for the deprecation change, but the sig feature matrix isn't linked until the very last section, so might be useful to link it sooner so that a new user can easily discover how to look-up the code-owner sig for the file(s) they are touching.

Ah good spot! I can add that here too, thanks for the feedback @cgalvan 👍

Actually I had a think about this and with the PR, the author should be notified of the code owner automatically, but I can make an explicit mention of this to be clearer

tonybalandiuk

looks good. If we have an example of a deprecation it may be worth referring to it with PR links to assist the community in understanding what an overall deprecation looks like from start to end. I'm thinking about this especially because of the 2-steps: "O3DE_DEPRECATION_NOTICE" followed by "AZ_DEPRECATED". This can always be added later.

Ulrick28

I would like to see the timeline resolved (ie, 1 release, 1 full release cycle, or 2-3 months) before I am willing to sign off on the plan. Otherwise, looks good.

hultonha · 2022-07-29T10:46:12Z

I would like to see the timeline resolved (ie, 1 release, 1 full release cycle, or 2-3 months) before I am willing to sign off on the plan. Otherwise, looks good.

That's fair, I think at this stage in the project we should go with 1 release right now which is largely what engineers have been doing already. If/when releases become more regular we could adjust this but I think that's a good starting option. I'll updating the wording to clarify.

Co-authored-by: William Hayward <[email protected]> Signed-off-by: Tom Hulton-Harrop <[email protected]>

Signed-off-by: Tom Hulton-Harrop <[email protected]>

hultonha · 2022-07-29T11:17:14Z

@willihay, @Ulrick28, @tonybalandiuk, @cgalvan - I've made some updates following your feedback. Once you're happy let me know and I'll get this merged. Thanks again for your time looking at this 👍

lmbr-pip · 2022-08-01T16:29:11Z

As an reader, IMHO the term deprecation can have a narrow view ie I should follow this guide when I am removing things, not when I am revising an API etc. Its also asking more cognitive process from non-english speakers.

Its probably healthier to have this focus on "Breaking changes ARE deprecation changes" ie deprecate the old thing, introduce the thing.

I would:

Have a lead in paragraph, that covers that "all breaking changes are deprecation changes", we want to as much as possible avoid breaking changes so you probably need to plan to deprecate old thing and introduce new thing whether its an API, a system or a data format.
Have a separate and highlighted, "this is how you should announce you are breaking something" section. Guide should also include letting impacted SIG(s) know in their Discord channels when possible. May want to also callout that if SIG(s) cannot reach agreement on deprecation approach, then issue should be raised with TSC. Ideally maintainers can link to this for PR approvals so we can nudge folks into following guide.
Format changes should also be in here, including provision of tooling to aid in format migration. Lots of code changes also introduce breaking format changes easily.

hultonha · 2022-08-03T10:33:53Z

As an reader, IMHO the term deprecation can have a narrow view ie I should follow this guide when I am removing things, not when I am revising an API etc. Its also asking more cognitive process from non-english speakers.

Its probably healthier to have this focus on "Breaking changes ARE deprecation changes" ie deprecate the old thing, introduce the thing.

I would:

* Have a lead in paragraph, that covers that "all breaking changes are deprecation changes", we want to as much as possible avoid breaking changes so you probably need to plan to deprecate old thing and introduce new thing whether its an API, a system or a data format.

* Have a separate and highlighted, "this is how you should announce you are breaking something" section. Guide should also include letting impacted SIG(s) know in their Discord channels when possible. May want to also callout that if SIG(s) cannot reach agreement on deprecation approach, then issue should be raised with TSC. Ideally maintainers can link to this for PR approvals so we can nudge folks into following guide.

* Format changes should also be in here, including provision of tooling to aid in format migration. Lots of code changes also introduce breaking format changes easily.

@lmbr-pip There is a list of things that constitute a breaking change (see API Deprecation - Examples very near the top). I can add a lead in paragraph but I personally don't feel this is very ambiguous. I feel the information on 'what to do' is largely covered already when reading the enumerated steps. I can mention the point about the TSC, that's a good point about possibly contentious issues. The format changes (for data formats to not be confused with code formatting) were also deemed to be out of scope at least for now for this doc. We do mention having a migration path as being important but I'd rather another team own transitioning/changing data formats separately.

I'll make a couple of the updates you mentioned when I get time but then I'd really like to merge this at least as version 1.0. I'd then be happy for to to create a follow-up PR to make some additional changes/updates. Thanks!

Signed-off-by: Tom Hulton-Harrop <[email protected]>

hultonha · 2022-08-12T11:49:36Z

@lmbr-pip I have made some additional changes regarding your feedback (see 205ea1f). I don't agree formatting changes should be included here, they are out of scope of the topic of deprecation. There is already information in the doc about how to deprecate things by notifying SIGs etc.

I would really appreciate if you could approve in the current state and if you would like to make further updates/additions please follow the process steps at the top of the doc. Thanks!

lmbr-pip · 2022-08-22T23:21:39Z

Am happy to disagree and commit on using deprecation but I still think its confusing and overly narrow. We really want folks to use this guide when renaming or adding breaking changes to an API, many folks are going to miss that this is "deprecation"

Yes, you provide rename as an example, but its halfway through the document.

I have already had this confusion with one developer who renamed an API without a) deprecating the old API and b) adding a new renamed API - they did not consider this to be a "deprecating" change as they weren't "removing" functionality but instead went straight to breaking the API. And yes, we can solve that through better education, but I want to call out the risks in starting with "deprecation" as the main action of this guide.

As to the notification guide, again, yes it exists, but most folks can't find it and so we do not follow it consistently. Linking and highlighting those expectations here would help clarify that.

hultonha · 2022-08-23T17:02:20Z

Thanks for the feedback @lmbr-pip. I understand your concern, so would your proposed solution to be to rename the document? That seems like the biggest point of contention right now is that right? We could call is O3DE Breaking Changes Guide instead? And keep most of the rest of the content the same? Thanks!

lmbr-pip · 2022-09-06T17:03:45Z

@hultonha I didn't see your comment. I think we should publish this as is, so we can start using the guide. We can rename after the fact (or cover with linking overview documentation)

hultonha · 2022-09-07T13:01:18Z

Okay thanks @lmbr-pip, I'm going to merge now. Thank you!

add initial pass of deprecation guide

82ef05a

Signed-off-by: Tom Hulton-Harrop <[email protected]>

hultonha requested review from sptramer, cgalvan, willihay, lsemp3d, lmbr-pip, Ulrick28, greerdv, srikappa-amzn, AMZN-daimini, tonybalandiuk and jeremyong-az July 27, 2022 10:57

hultonha mentioned this pull request Jul 27, 2022

feat(AZCore): add CreateFromEuler/Radians/Degrees(XYZ,YXZ,ZYX) o3de/o3de#10852

Merged

willihay reviewed Jul 27, 2022

View reviewed changes

Update guides/deprecation.md

f09a0eb

Add apostrophe Co-authored-by: William Hayward <[email protected]> Signed-off-by: Tom Hulton-Harrop <[email protected]>

hultonha force-pushed the hultonha_deprecation-guide branch from a049261 to f09a0eb Compare July 27, 2022 16:47

cgalvan approved these changes Jul 27, 2022

View reviewed changes

tonybalandiuk approved these changes Jul 27, 2022

View reviewed changes

Ulrick28 reviewed Jul 27, 2022

View reviewed changes

hultonha added 2 commits July 29, 2022 11:49

Update guides/deprecation.md

ca6b74b

Co-authored-by: William Hayward <[email protected]> Signed-off-by: Tom Hulton-Harrop <[email protected]>

updates following review feedback

0925b47

Signed-off-by: Tom Hulton-Harrop <[email protected]>

hultonha force-pushed the hultonha_deprecation-guide branch from 7b214ce to 0925b47 Compare July 29, 2022 11:10

add missing full-stop

0f24885

Signed-off-by: Tom Hulton-Harrop <[email protected]>

willihay approved these changes Jul 29, 2022

View reviewed changes

updates after review feedback

205ea1f

Signed-off-by: Tom Hulton-Harrop <[email protected]>

lmbr-pip approved these changes Aug 22, 2022

View reviewed changes

hultonha merged commit 26361c6 into o3de:main Sep 7, 2022

hultonha deleted the hultonha_deprecation-guide branch September 7, 2022 13:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial pass of deprecation guide post RFC #141

Add initial pass of deprecation guide post RFC #141

hultonha commented Jul 27, 2022 •

edited

Loading

willihay left a comment •

edited

Loading

willihay Jul 27, 2022

hultonha Jul 27, 2022

willihay Jul 27, 2022

Ulrick28 Jul 27, 2022 •

edited

Loading

hultonha commented Jul 27, 2022

cgalvan Jul 27, 2022

hultonha Jul 29, 2022

hultonha Jul 29, 2022

tonybalandiuk left a comment

Ulrick28 left a comment

hultonha commented Jul 29, 2022

hultonha commented Jul 29, 2022

lmbr-pip commented Aug 1, 2022

hultonha commented Aug 3, 2022

hultonha commented Aug 12, 2022

lmbr-pip commented Aug 22, 2022 •

edited

Loading

hultonha commented Aug 23, 2022

lmbr-pip commented Sep 6, 2022

hultonha commented Sep 7, 2022

Add initial pass of deprecation guide post RFC #141

Add initial pass of deprecation guide post RFC #141

Conversation

hultonha commented Jul 27, 2022 • edited Loading

willihay left a comment • edited Loading

Choose a reason for hiding this comment

willihay Jul 27, 2022

Choose a reason for hiding this comment

hultonha Jul 27, 2022

Choose a reason for hiding this comment

willihay Jul 27, 2022

Choose a reason for hiding this comment

Ulrick28 Jul 27, 2022 • edited Loading

Choose a reason for hiding this comment

hultonha commented Jul 27, 2022

cgalvan Jul 27, 2022

Choose a reason for hiding this comment

hultonha Jul 29, 2022

Choose a reason for hiding this comment

hultonha Jul 29, 2022

Choose a reason for hiding this comment

tonybalandiuk left a comment

Choose a reason for hiding this comment

Ulrick28 left a comment

Choose a reason for hiding this comment

hultonha commented Jul 29, 2022

hultonha commented Jul 29, 2022

lmbr-pip commented Aug 1, 2022

hultonha commented Aug 3, 2022

hultonha commented Aug 12, 2022

lmbr-pip commented Aug 22, 2022 • edited Loading

hultonha commented Aug 23, 2022

lmbr-pip commented Sep 6, 2022

hultonha commented Sep 7, 2022

hultonha commented Jul 27, 2022 •

edited

Loading

willihay left a comment •

edited

Loading

Ulrick28 Jul 27, 2022 •

edited

Loading

lmbr-pip commented Aug 22, 2022 •

edited

Loading