From 97681cf9fe91a86fe06b9158b45c684ca615e135 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:28:17 -0800 Subject: [PATCH 01/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index a384618b..c439400e 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -345,25 +345,25 @@ In general, making all but additive changes to the API contract for existing ele **Non-breaking changes:** -- To add properties that are nullable or have a default value +- To add properties that are nullable or have a default value //// TODO this sort of contradicts the nullable=false point below; also we should make clear that defaults don't necessarily have to come from the csdl - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs -- Adding or removing an annotation OpenType="true" +- Adding or removing an annotation OpenType="true" //// isn't closing a type a breaking change for write APIs? **Breaking changes:** - Changing the URL or fundamental request/response associated with a resource -- Removing, renaming, or changing an incompatible type of a declared property +- Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? - Removing or renaming APIs or API parameters - Adding a required request header - Adding EnumType members for nonevolvable enumerations - Adding Nullable="false" properties to existing types -- Adding a parameter not marked as [Nullable](http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Nullable) to existing actions +- Adding a parameter not marked as [Nullable](http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Nullable) to existing actions //// TODO null doesn't always mean the same thing as "default"; would action overloads be more appropriate for these cases? - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function -- Changing top-level error codes -- Introducing server-side pagination to existing collections +- Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? +- Introducing server-side pagination to existing collections //// TODO do we have an established pattern to introduce server-side pagination to existing collections? - Making significant changes to the performance of APIs such as increased latency, rate limits, or concurrency The applicable changes described in the [Model Versioning of the OData V4.01 spec](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_ModelVersioning) SHOULD be considered part of the minimum bar that all services MUST consider a breaking change. From 70028ecd1422bd18a25b1bf08caec7c964c70305 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 12 Jan 2024 16:29:15 -0800 Subject: [PATCH 02/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index c439400e..a470a09d 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -351,6 +351,8 @@ In general, making all but additive changes to the API contract for existing ele - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs - Adding or removing an annotation OpenType="true" //// isn't closing a type a breaking change for write APIs? +//// TODO add notes that introducing type hierarchies aren't breaking changes? +//// TODO add notes that returning new derived tpyes in a collection are/aren't breaking changes? **Breaking changes:** From 5cb4823fcd69574c14c86fff169c74c49d4741e3 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 17 Jan 2024 10:35:30 -0800 Subject: [PATCH 03/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index a470a09d..622fe2e6 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -345,6 +345,10 @@ In general, making all but additive changes to the API contract for existing ele **Non-breaking changes:** +//// TODO merge rest guidelines into this +//// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support#api-contract-and-nonbackward-compatible-changes +//// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support + - To add properties that are nullable or have a default value //// TODO this sort of contradicts the nullable=false point below; also we should make clear that defaults don't necessarily have to come from the csdl - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation From dea47dbae8738a42d89f95d0c2c6a39cfaf61753 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 17 Jan 2024 11:01:18 -0800 Subject: [PATCH 04/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 622fe2e6..c99f0922 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -355,7 +355,34 @@ In general, making all but additive changes to the API contract for existing ele - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs - Adding or removing an annotation OpenType="true" //// isn't closing a type a breaking change for write APIs? -//// TODO add notes that introducing type hierarchies aren't breaking changes? +//// sdk: +//// removing open type and don't schematize: the property will remain in the dynamic properties collection, so no issue +//// removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now + +//// TODO differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break +//// TODO this doc should be about the rest api and not necessarily about the sdk (maybe a different doc for those? that's a whole other topic though) + +PATCH /someEntity +{ + "foo": "...", + "dynamicProperty": "..." +} + + +PATCH /someEntity +{ + "foo": "...", + "dynamicProperty": "..." +} +//// whenever you close an open type, all dyanmic properties should be schematized + + +//// TODO add notes that expanding type hierarchies aren't breaking changes? +//// ^ sdks can have issues + + +//// TODO this is where we left off +//// TODO add notes that base types type hierarchies aren't breaking changes? //// TODO add notes that returning new derived tpyes in a collection are/aren't breaking changes? **Breaking changes:** From 003d7db5eec3b5fa4616ff64a47b939ba8ab2001 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:40:49 -0800 Subject: [PATCH 05/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index c99f0922..f044fc86 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -350,6 +350,8 @@ In general, making all but additive changes to the API contract for existing ele //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support - To add properties that are nullable or have a default value //// TODO this sort of contradicts the nullable=false point below; also we should make clear that defaults don't necessarily have to come from the csdl +//// TODO non-nullable values aren't breaking if they have a server-provided default value +//// TODO this is orthogonal to required for creation properties; link to the nullable doc - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation - Changing the order of properties From e879a7ef2f5d161f4745777f4933828c6c44c374 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:43:55 -0800 Subject: [PATCH 06/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index f044fc86..7ee0b05e 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -361,6 +361,9 @@ In general, making all but additive changes to the API contract for existing ele //// removing open type and don't schematize: the property will remain in the dynamic properties collection, so no issue //// removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now + +//// TODO adding new properties that are required for creation is breaking + //// TODO differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break //// TODO this doc should be about the rest api and not necessarily about the sdk (maybe a different doc for those? that's a whole other topic though) From 0283dd008cfad53af99d8c12217f12325d540954 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 31 Jan 2024 10:50:16 -0800 Subject: [PATCH 07/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 1 + 1 file changed, 1 insertion(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 7ee0b05e..b10c5bfc 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -394,6 +394,7 @@ PATCH /someEntity - Changing the URL or fundamental request/response associated with a resource - Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? +//// TODO the compatible types are still lieklly to be breaking sdk changes for some languages - Removing or renaming APIs or API parameters - Adding a required request header - Adding EnumType members for nonevolvable enumerations From 9940b8093c062fcc1e68947a05f9052efbc5b2ed Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 31 Jan 2024 11:01:26 -0800 Subject: [PATCH 08/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index b10c5bfc..63c54b9c 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -354,6 +354,20 @@ In general, making all but additive changes to the API contract for existing ele //// TODO this is orthogonal to required for creation properties; link to the nullable doc - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation +//// TODO gareth has mentioned that tooling relies on annotations, so these *should* be considered breaking changes +//// TODO this is actually about instance annotations; this needs to be discussed further +//// TODO follow up on both topics with gareth and mike + +"foo@microsoft.old" //// removing this should not be allowed depending on if it was always being returned vs contextually returned vs etc +"foo@microsoft.new" //// adding this should be allowed +//// TODO overall, the instanace annotations thing shuold follow the same guidance as properties +//// you can select instance annotations (if marked up this way), so removing even a contextually returned one is a break in those cases + +//// control information that was always returned by default should not be removed from the payload +//// type of instance annotations should definitely not change (or should follow the other guidance) + + +//// TODO we should further discuss this same thing but about model annotations - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs - Adding or removing an annotation OpenType="true" //// isn't closing a type a breaking change for write APIs? From 48e9308a22278812ce6baa08b8689fe8e1762468 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:18:30 -0800 Subject: [PATCH 09/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 63c54b9c..e7220e7f 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -349,9 +349,15 @@ In general, making all but additive changes to the API contract for existing ele //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support#api-contract-and-nonbackward-compatible-changes //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support -- To add properties that are nullable or have a default value //// TODO this sort of contradicts the nullable=false point below; also we should make clear that defaults don't necessarily have to come from the csdl -//// TODO non-nullable values aren't breaking if they have a server-provided default value -//// TODO this is orthogonal to required for creation properties; link to the nullable doc +option 1 +- Adding a property that has a default value (NOTE: a new property that is marked `Nullable="false"` and does not have a `DefaultValue` attribute specified may still have a service-provided default value, though a suppression will be required in this case; also note that [nullability and default values are orthogonal](https://github.com/microsoft/api-guidelines/blob/corranrogue9/breakingchanges/graph/articles/nullable.md) to properties that are required for creation) + +option 2 +- Adding a property that is marked `Nullable="true"` +- Adding a property that is marked with the `DefaultValue` attribute +- Adding a property that has a service-provided default value (a suppression will be required in this case) + + - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation //// TODO gareth has mentioned that tooling relies on annotations, so these *should* be considered breaking changes From c8a1cce8e86863d299cfc15cc198fd3f9410e29a Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:19:17 -0800 Subject: [PATCH 10/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index e7220e7f..7f7cdf03 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -382,8 +382,6 @@ option 2 //// removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now -//// TODO adding new properties that are required for creation is breaking - //// TODO differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break //// TODO this doc should be about the rest api and not necessarily about the sdk (maybe a different doc for those? that's a whole other topic though) @@ -412,6 +410,7 @@ PATCH /someEntity **Breaking changes:** +- Adding a property that is required for the creation of the type it is defined on - Changing the URL or fundamental request/response associated with a resource - Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? //// TODO the compatible types are still lieklly to be breaking sdk changes for some languages From 4cb38c4ad34b9de4716b9b5692af2b7e007cab13 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:20:27 -0800 Subject: [PATCH 11/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 7f7cdf03..9d9e4192 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -360,20 +360,21 @@ option 2 - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation -//// TODO gareth has mentioned that tooling relies on annotations, so these *should* be considered breaking changes -//// TODO this is actually about instance annotations; this needs to be discussed further -//// TODO follow up on both topics with gareth and mike +/* TODO +gareth has mentioned that tooling relies on annotations, so these *should* be considered breaking changes +this is actually about instance annotations; this needs to be discussed further +follow up on both topics with gareth and mike "foo@microsoft.old" //// removing this should not be allowed depending on if it was always being returned vs contextually returned vs etc "foo@microsoft.new" //// adding this should be allowed -//// TODO overall, the instanace annotations thing shuold follow the same guidance as properties -//// you can select instance annotations (if marked up this way), so removing even a contextually returned one is a break in those cases -//// control information that was always returned by default should not be removed from the payload -//// type of instance annotations should definitely not change (or should follow the other guidance) +overall, the instanace annotations thing shuold follow the same guidance as properties +you can select instance annotations (if marked up this way), so removing even a contextually returned one is a break in those cases +control information that was always returned by default should not be removed from the payload +type of instance annotations should definitely not change (or should follow the other guidance) +*/ -//// TODO we should further discuss this same thing but about model annotations - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs - Adding or removing an annotation OpenType="true" //// isn't closing a type a breaking change for write APIs? @@ -403,6 +404,7 @@ PATCH /someEntity //// TODO add notes that expanding type hierarchies aren't breaking changes? //// ^ sdks can have issues +//// TODO we should further discuss model annotations //// TODO this is where we left off //// TODO add notes that base types type hierarchies aren't breaking changes? From bff2ccc052eb606dc26a83698652fe56bc73cf09 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:21:58 -0800 Subject: [PATCH 12/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 9d9e4192..bbe0cdfe 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -349,6 +349,12 @@ In general, making all but additive changes to the API contract for existing ele //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support#api-contract-and-nonbackward-compatible-changes //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support +/*TODO sdk breaks +differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break: +removing open type and don't schematize: the property will remain in the dynamic properties collection, so no issue +removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now +*/ + option 1 - Adding a property that has a default value (NOTE: a new property that is marked `Nullable="false"` and does not have a `DefaultValue` attribute specified may still have a service-provided default value, though a suppression will be required in this case; also note that [nullability and default values are orthogonal](https://github.com/microsoft/api-guidelines/blob/corranrogue9/breakingchanges/graph/articles/nullable.md) to properties that are required for creation) @@ -378,13 +384,10 @@ type of instance annotations should definitely not change (or should follow the - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs - Adding or removing an annotation OpenType="true" //// isn't closing a type a breaking change for write APIs? -//// sdk: -//// removing open type and don't schematize: the property will remain in the dynamic properties collection, so no issue -//// removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now -//// TODO differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break -//// TODO this doc should be about the rest api and not necessarily about the sdk (maybe a different doc for those? that's a whole other topic though) + + PATCH /someEntity { From ba2bcd0253a2b61a24171c70298438b0a025efde Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:23:55 -0800 Subject: [PATCH 13/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index bbe0cdfe..021bfc96 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -383,9 +383,9 @@ type of instance annotations should definitely not change (or should follow the - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs -- Adding or removing an annotation OpenType="true" //// isn't closing a type a breaking change for write APIs? - - +- Adding the `OpenType="true"` attribute +- Removing the `OpenType="true"` attribute for read-only APIs +- Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change From 4c305a8f08a898d1946a76c3d27617377325a36f Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:24:46 -0800 Subject: [PATCH 14/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 1 + 1 file changed, 1 insertion(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 021bfc96..7e22d888 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -363,6 +363,7 @@ option 2 - Adding a property that is marked with the `DefaultValue` attribute - Adding a property that has a service-provided default value (a suppression will be required in this case) +--end of options - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation From 228f88f1cef75d856d148a8b25fc777af1e9f150 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:26:44 -0800 Subject: [PATCH 15/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 27 ++++----------------------- 1 file changed, 4 insertions(+), 23 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 7e22d888..e166e163 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -353,6 +353,7 @@ In general, making all but additive changes to the API contract for existing ele differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break: removing open type and don't schematize: the property will remain in the dynamic properties collection, so no issue removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now +sdks can have issues with expanding type hierarchies */ option 1 @@ -361,9 +362,8 @@ option 1 option 2 - Adding a property that is marked `Nullable="true"` - Adding a property that is marked with the `DefaultValue` attribute -- Adding a property that has a service-provided default value (a suppression will be required in this case) +- Adding a property that has a service-provided default value (a suppression will be required in this case) ---end of options - Adding a member after the sentinel member to an evolvable enumeration - Removing, renaming, or changing the type of annotation @@ -389,28 +389,9 @@ type of instance annotations should definitely not change (or should follow the - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change - -PATCH /someEntity -{ - "foo": "...", - "dynamicProperty": "..." -} - - -PATCH /someEntity -{ - "foo": "...", - "dynamicProperty": "..." -} -//// whenever you close an open type, all dyanmic properties should be schematized - - -//// TODO add notes that expanding type hierarchies aren't breaking changes? -//// ^ sdks can have issues - -//// TODO we should further discuss model annotations - //// TODO this is where we left off +//// TODO we should further discuss model annotations +//// TODO add notes that expanding type hierarchies aren't breaking changes //// TODO add notes that base types type hierarchies aren't breaking changes? //// TODO add notes that returning new derived tpyes in a collection are/aren't breaking changes? From 14335c15de5ada8a1190221d74c88ca6a71ed8aa Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:30:13 -0800 Subject: [PATCH 16/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index e166e163..fad8e95e 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -366,6 +366,13 @@ option 2 - Adding a member after the sentinel member to an evolvable enumeration +- Adding new instance annotations to a response payload //// TODO link to docs +- Adding control information to a response payload //// TODO link to docs +- Removing an instance annotaiton from a response payload if the annotation is not selectable and the current behavior is that the annotation is not always present in the response payload + + + + - Removing, renaming, or changing the type of annotation /* TODO gareth has mentioned that tooling relies on annotations, so these *should* be considered breaking changes From 41588e84dead41040361c4819125e77a738eb437 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:33:47 -0800 Subject: [PATCH 17/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index fad8e95e..c9bb4b91 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -369,26 +369,10 @@ option 2 - Adding new instance annotations to a response payload //// TODO link to docs - Adding control information to a response payload //// TODO link to docs - Removing an instance annotaiton from a response payload if the annotation is not selectable and the current behavior is that the annotation is not always present in the response payload +- Removing control information from a response payload if the current behavior is that the control information is not always present in the response payload - -- Removing, renaming, or changing the type of annotation -/* TODO -gareth has mentioned that tooling relies on annotations, so these *should* be considered breaking changes -this is actually about instance annotations; this needs to be discussed further -follow up on both topics with gareth and mike - -"foo@microsoft.old" //// removing this should not be allowed depending on if it was always being returned vs contextually returned vs etc -"foo@microsoft.new" //// adding this should be allowed - -overall, the instanace annotations thing shuold follow the same guidance as properties -you can select instance annotations (if marked up this way), so removing even a contextually returned one is a break in those cases - -control information that was always returned by default should not be removed from the payload -type of instance annotations should definitely not change (or should follow the other guidance) -*/ - - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs - Adding the `OpenType="true"` attribute @@ -404,6 +388,10 @@ type of instance annotations should definitely not change (or should follow the **Breaking changes:** +- Changing the type of an instance annotation +- Removing an instance annotation from a response paylod if the annotation is selectable +- Removing an instance annotaiton from a response payload if the current behavior is that the annotation is always present in the response payload +- Removing control information from a response payload if the current behavior is that the control information is always present in the response payload - Adding a property that is required for the creation of the type it is defined on - Changing the URL or fundamental request/response associated with a resource - Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? From bc78f37da53f540aefe133cfa5d183c196a5469b Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 2 Feb 2024 08:35:30 -0800 Subject: [PATCH 18/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index c9bb4b91..671cc937 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -364,27 +364,22 @@ option 2 - Adding a property that is marked with the `DefaultValue` attribute - Adding a property that has a service-provided default value (a suppression will be required in this case) - - Adding a member after the sentinel member to an evolvable enumeration - Adding new instance annotations to a response payload //// TODO link to docs - Adding control information to a response payload //// TODO link to docs - Removing an instance annotaiton from a response payload if the annotation is not selectable and the current behavior is that the annotation is not always present in the response payload - Removing control information from a response payload if the current behavior is that the control information is not always present in the response payload - - - - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs - Adding the `OpenType="true"` attribute - Removing the `OpenType="true"` attribute for read-only APIs - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change - -//// TODO this is where we left off -//// TODO we should further discuss model annotations //// TODO add notes that expanding type hierarchies aren't breaking changes //// TODO add notes that base types type hierarchies aren't breaking changes? //// TODO add notes that returning new derived tpyes in a collection are/aren't breaking changes? +//// TODO this is where we left off +//// TODO we should further discuss model annotations **Breaking changes:** From 88e7475f87b6a749aea23d9cdfb188db73ca7c09 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 7 Feb 2024 11:00:21 -0800 Subject: [PATCH 19/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 76 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 671cc937..2b5e6513 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -371,13 +371,85 @@ option 2 - Removing control information from a response payload if the current behavior is that the control information is not always present in the response payload - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs -- Adding the `OpenType="true"` attribute +- Adding the `OpenType="true"` attribute //// TODO remove this? linnk to a place where we say you can't have open types; maybehave a non-recommending non-breaking changes - Removing the `OpenType="true"` attribute for read-only APIs - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change + +//// TODO expanded type hierarchies are source or binary compatibility issues //// TODO add notes that expanding type hierarchies aren't breaking changes + +starting: + +foo + prop3 + prop4 + +bar : foo + prop1 + prop2 + +next release + +base + prop3 + +foo : base + prop4 + +bar : foo + prop1 + prop2 + + +-> +[ + { + "prop3": + "prop4": + } +] + +TODO is it a break to add the below? the decision is that it is not a break +TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner + +otherbar : foo + prop7 + prop8 + + + //// TODO add notes that base types type hierarchies aren't breaking changes? -//// TODO add notes that returning new derived tpyes in a collection are/aren't breaking changes? + +starting: + +foo + prop3 + prop4 + +bar : foo + prop1 + prop2 + + + +next release + +foo + +intermediate : foo + prop3 + prop4 + +bar : intermediate + prop1 + prop2 + + //// TODo this is an sdk break + + + + //// TODO this is where we left off //// TODO we should further discuss model annotations From ed018f54f37f12492be3c5c2019f01d9426b360c Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:19:51 -0800 Subject: [PATCH 20/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 1 + 1 file changed, 1 insertion(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 2b5e6513..1c2a0d55 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -348,6 +348,7 @@ In general, making all but additive changes to the API contract for existing ele //// TODO merge rest guidelines into this //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support#api-contract-and-nonbackward-compatible-changes //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support +//// TODO update verbiage of sdk vs api versions: https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707321601369?context=%7B%22contextType%22%3A%22chat%22%7D /*TODO sdk breaks differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break: From e045375cfd885cdfc0c741cee610edd6172b241a Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:26:31 -0800 Subject: [PATCH 21/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 35 ++--------------------------------- 1 file changed, 2 insertions(+), 33 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 1c2a0d55..807880aa 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -355,6 +355,7 @@ differentiate between sdk vs rest breaking changes; also differentiate if client removing open type and don't schematize: the property will remain in the dynamic properties collection, so no issue removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now sdks can have issues with expanding type hierarchies +adding a new base type (regardless of if referencing properties are changed) */ option 1 @@ -375,41 +376,9 @@ option 2 - Adding the `OpenType="true"` attribute //// TODO remove this? linnk to a place where we say you can't have open types; maybehave a non-recommending non-breaking changes - Removing the `OpenType="true"` attribute for read-only APIs - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change +- Adding a new base type provided that no property `Type` attributes are changed -//// TODO expanded type hierarchies are source or binary compatibility issues -//// TODO add notes that expanding type hierarchies aren't breaking changes - -starting: - -foo - prop3 - prop4 - -bar : foo - prop1 - prop2 - -next release - -base - prop3 - -foo : base - prop4 - -bar : foo - prop1 - prop2 - - --> -[ - { - "prop3": - "prop4": - } -] TODO is it a break to add the below? the decision is that it is not a break TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner From 45c3029004908c93e2ae2f19d1e8ba5c53bae409 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:27:46 -0800 Subject: [PATCH 22/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 807880aa..beffcdd8 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -376,16 +376,8 @@ option 2 - Adding the `OpenType="true"` attribute //// TODO remove this? linnk to a place where we say you can't have open types; maybehave a non-recommending non-breaking changes - Removing the `OpenType="true"` attribute for read-only APIs - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change -- Adding a new base type provided that no property `Type` attributes are changed - - - -TODO is it a break to add the below? the decision is that it is not a break -TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner - -otherbar : foo - prop7 - prop8 +- Adding a new base type to an existing type provided that no property `Type` attributes are changed +- Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner From a4be78470b70f7c53a0bd5f01b2087b3c9107ee0 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:34:28 -0800 Subject: [PATCH 23/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 38 ++++---------------------------------- 1 file changed, 4 insertions(+), 34 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index beffcdd8..c61efa8f 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -356,6 +356,7 @@ removing open type and don't schematize: the property will remain in the dynamic removing open type and schematizing: the schematized property will no longer be in the dynamic properties colleciton, so if a client is looking for it in the collection, they won't find it now sdks can have issues with expanding type hierarchies adding a new base type (regardless of if referencing properties are changed) +adding an intermediate type */ option 1 @@ -376,43 +377,12 @@ option 2 - Adding the `OpenType="true"` attribute //// TODO remove this? linnk to a place where we say you can't have open types; maybehave a non-recommending non-breaking changes - Removing the `OpenType="true"` attribute for read-only APIs - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change -- Adding a new base type to an existing type provided that no property `Type` attributes are changed +- Adding a new base type to an existing type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing type into the base type - Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner - - - -//// TODO add notes that base types type hierarchies aren't breaking changes? - -starting: - -foo - prop3 - prop4 - -bar : foo - prop1 - prop2 - - - -next release - -foo - -intermediate : foo - prop3 - prop4 - -bar : intermediate - prop1 - prop2 - - //// TODo this is an sdk break - - - +- Adding a new type in the inheritance hierarchy between an existing type and its current base type provided that no property `Type` attributes are changed that currently reference the existing type; this include moving properties from the existing child type into the new base type //// TODO this is where we left off +//// TODO during the last discussion, the above was said to require a change to the type attributes of properties; i don't think this is the case //// TODO we should further discuss model annotations **Breaking changes:** From fed19d5cf3d948062e320ce4ccbfd2586ba41d7b Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 8 Feb 2024 09:35:19 -0800 Subject: [PATCH 24/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index c61efa8f..61b052ef 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -378,7 +378,7 @@ option 2 - Removing the `OpenType="true"` attribute for read-only APIs - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change - Adding a new base type to an existing type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing type into the base type -- Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner +- Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner; TODO follow up if this is an SDK break https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707412726923?context=%7B%22contextType%22%3A%22chat%22%7D - Adding a new type in the inheritance hierarchy between an existing type and its current base type provided that no property `Type` attributes are changed that currently reference the existing type; this include moving properties from the existing child type into the new base type //// TODO this is where we left off From 0930b38eae254ab411f5bf75495ae609b10768e4 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 14 Feb 2024 10:59:58 -0800 Subject: [PATCH 25/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 55 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 61b052ef..008a4f80 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -379,7 +379,60 @@ option 2 - Removing the `OpenType="true"` attribute for write APIs if all of the possible dynamic properties are also schematized in the same change - Adding a new base type to an existing type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing type into the base type - Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner; TODO follow up if this is an SDK break https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707412726923?context=%7B%22contextType%22%3A%22chat%22%7D -- Adding a new type in the inheritance hierarchy between an existing type and its current base type provided that no property `Type` attributes are changed that currently reference the existing type; this include moving properties from the existing child type into the new base type +- Adding a new type in the inheritance hierarchy between an existing type and its current base type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing child type into the new base type + +v1 +{ +foo + prop1 + prop2 + +bar : foo + prop3 + prop4 + +foos collection(foo) +bars collection(bar) +} + +v2 +{ +foo + prop1 + prop2 + +intermediate : foo //// TODO can't move prop2 to intermediate without a change to the foos collection + prop3 + +bar : intermediate + prop4 + +foos collection(foo) +bars collection(bar) +} + +v3 +{ +foo + prop1 + +intermediate : foo + prop3 + prop2 + +bar : intermediate + prop4 + +foos collection(intermediate) +bars collection(bar) +} + +POST .../foos +{ +//// TODO this will now fail, it's a break + "@odata.type": "#...foo", +} + //// TODO this is where we left off //// TODO during the last discussion, the above was said to require a change to the type attributes of properties; i don't think this is the case From f645530265f212f12c68505377b9df9c1fe4e347 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 16 Feb 2024 08:44:22 -0800 Subject: [PATCH 26/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 008a4f80..5e234584 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -436,6 +436,7 @@ POST .../foos //// TODO this is where we left off //// TODO during the last discussion, the above was said to require a change to the type attributes of properties; i don't think this is the case +//// TODO examples of each of these? //// TODO we should further discuss model annotations **Breaking changes:** @@ -457,6 +458,7 @@ POST .../foos - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? - Introducing server-side pagination to existing collections //// TODO do we have an established pattern to introduce server-side pagination to existing collections? - Making significant changes to the performance of APIs such as increased latency, rate limits, or concurrency +//// TODO make clear any of the "add stuff to the type hierarchy" changes that *are* breaking? e.g. adding an intermediate type and changing a `Type` attribute (breaks clients that are currently using `@odata.type`), adding an intermediate type and moving a property from the base type to the new type, etc.? The applicable changes described in the [Model Versioning of the OData V4.01 spec](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_ModelVersioning) SHOULD be considered part of the minimum bar that all services MUST consider a breaking change. From 4dd1c54145da7fd1893553cd1960019d842c6171 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 16 Feb 2024 08:47:06 -0800 Subject: [PATCH 27/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 55 +--------------------------------------- 1 file changed, 1 insertion(+), 54 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 5e234584..d5a77986 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -381,61 +381,7 @@ option 2 - Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner; TODO follow up if this is an SDK break https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707412726923?context=%7B%22contextType%22%3A%22chat%22%7D - Adding a new type in the inheritance hierarchy between an existing type and its current base type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing child type into the new base type -v1 -{ -foo - prop1 - prop2 - -bar : foo - prop3 - prop4 - -foos collection(foo) -bars collection(bar) -} - -v2 -{ -foo - prop1 - prop2 - -intermediate : foo //// TODO can't move prop2 to intermediate without a change to the foos collection - prop3 - -bar : intermediate - prop4 - -foos collection(foo) -bars collection(bar) -} - -v3 -{ -foo - prop1 - -intermediate : foo - prop3 - prop2 - -bar : intermediate - prop4 - -foos collection(intermediate) -bars collection(bar) -} - -POST .../foos -{ -//// TODO this will now fail, it's a break - "@odata.type": "#...foo", -} - - //// TODO this is where we left off -//// TODO during the last discussion, the above was said to require a change to the type attributes of properties; i don't think this is the case //// TODO examples of each of these? //// TODO we should further discuss model annotations @@ -458,6 +404,7 @@ POST .../foos - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? - Introducing server-side pagination to existing collections //// TODO do we have an established pattern to introduce server-side pagination to existing collections? - Making significant changes to the performance of APIs such as increased latency, rate limits, or concurrency +//// TODO related to the below comment, do we really want to specify what are breaking changes? aren't we really just creating a contract with client developers saying what are acceptable changes so that they can code defensively for them? i understand that it will limit us/make this document invalid if we ever need to do something that's *not* enumerated (like a header or something), but if we don't take a strong stance on this, then what's the point of the doc at all? //// TODO make clear any of the "add stuff to the type hierarchy" changes that *are* breaking? e.g. adding an intermediate type and changing a `Type` attribute (breaks clients that are currently using `@odata.type`), adding an intermediate type and moving a property from the base type to the new type, etc.? The applicable changes described in the [Model Versioning of the OData V4.01 spec](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_ModelVersioning) SHOULD be considered part of the minimum bar that all services MUST consider a breaking change. From f5909e96c2b1439a058bb017509fa39709bc40e7 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:38:57 -0800 Subject: [PATCH 28/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index d5a77986..c04e6c97 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -349,6 +349,7 @@ In general, making all but additive changes to the API contract for existing ele //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support#api-contract-and-nonbackward-compatible-changes //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support //// TODO update verbiage of sdk vs api versions: https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707321601369?context=%7B%22contextType%22%3A%22chat%22%7D +//// TODO we should further discuss model annotations? we will not document these as breaks today; we will kick this down the road for when we are using typespec and the annotations that graph is publishing are actually accurate /*TODO sdk breaks differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break: @@ -370,7 +371,7 @@ option 2 - Adding a member after the sentinel member to an evolvable enumeration - Adding new instance annotations to a response payload //// TODO link to docs - Adding control information to a response payload //// TODO link to docs -- Removing an instance annotaiton from a response payload if the annotation is not selectable and the current behavior is that the annotation is not always present in the response payload +- Removing an instance annotation from a response payload if the annotation is not selectable and the current behavior is that the annotation is not always present in the response payload - Removing control information from a response payload if the current behavior is that the control information is not always present in the response payload - Changing the order of properties - Changing the length or format of opaque strings, such as resource IDs @@ -381,9 +382,7 @@ option 2 - Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner; TODO follow up if this is an SDK break https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707412726923?context=%7B%22contextType%22%3A%22chat%22%7D - Adding a new type in the inheritance hierarchy between an existing type and its current base type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing child type into the new base type -//// TODO this is where we left off -//// TODO examples of each of these? -//// TODO we should further discuss model annotations +//// TODO ahve a breaking change article that has samples, and in this doc have hihg level categories of brekaing changes **Breaking changes:** From 10d6ef81c52921fdcef8c6655df4af3ef0711614 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 21 Feb 2024 10:59:38 -0800 Subject: [PATCH 29/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index c04e6c97..d6d3ba52 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -394,13 +394,39 @@ option 2 - Changing the URL or fundamental request/response associated with a resource - Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? //// TODO the compatible types are still lieklly to be breaking sdk changes for some languages + +//// TODO actually come up with samples for the above + - Removing or renaming APIs or API parameters - Adding a required request header - Adding EnumType members for nonevolvable enumerations - Adding Nullable="false" properties to existing types - Adding a parameter not marked as [Nullable](http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Nullable) to existing actions //// TODO null doesn't always mean the same thing as "default"; would action overloads be more appropriate for these cases? + + + + + + + + +//// TODO can you overload actions? apparently not... +//// TODO can we use the optionalparamter annotation? yes, we should suggest this +//// TODO check linting rules are ok with this guidance before shipping it - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? +//// TODO 5xx can be changed to 4xx +//// TODO cannot within the 4xx's +//// can you change within 5xx's? +//// clients should never code against 400 specifically; in these branches, they should check for *any* 4xx +//// TODO do we want to require that innererrors remain the same? + +//// TODO have success, redirect, and error categories +//// can't change 200 to 202 + +//// TODO what about http status codes? the top-level error codes are expected to be the same as http status codes (is this guidance actually written?) + +//// TODO beginning the use of redirects should be ok - Introducing server-side pagination to existing collections //// TODO do we have an established pattern to introduce server-side pagination to existing collections? - Making significant changes to the performance of APIs such as increased latency, rate limits, or concurrency //// TODO related to the below comment, do we really want to specify what are breaking changes? aren't we really just creating a contract with client developers saying what are acceptable changes so that they can code defensively for them? i understand that it will limit us/make this document invalid if we ever need to do something that's *not* enumerated (like a header or something), but if we don't take a strong stance on this, then what's the point of the doc at all? From 40117d22a1522234afd5b66612a90d35ad3f7758 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 22 Feb 2024 10:23:56 -0800 Subject: [PATCH 30/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index d6d3ba52..87e6d416 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -395,7 +395,36 @@ option 2 - Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? //// TODO the compatible types are still lieklly to be breaking sdk changes for some languages -//// TODO actually come up with samples for the above + + + + + + + + + + + + + + + +GET /containers + +{ + "value": [ + { + "id": "{containerId1}", + "propName": { + } + } + ] +} + +//// I don't really see how you can change the type of `propName`. If you make it `foo`, then any `intermediate`s that are returned now have an odata.type that clients need to check. If you make it `bar`, then `intermediate`s can no longer be returned; further, if you only ever returned `bar`s before, they now wouldn't have the `@odata.type`. +//// On the writing side of things, it seems like you could make `propName` into `foo`, except that now, previous requests would fail because they don't have `@odata.type` of `intermediate`. Changing it to `bar` would now prevent the `intermediate`s from being written entirely. +//// Does anyone have examples of when change the `Type` attribute of a property is allowed? - Removing or renaming APIs or API parameters - Adding a required request header From b8220ed124488fde2f760e240cb9110a22630ee2 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 22 Feb 2024 10:46:40 -0800 Subject: [PATCH 31/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 87e6d416..fefa075b 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -432,6 +432,13 @@ GET /containers - Adding Nullable="false" properties to existing types - Adding a parameter not marked as [Nullable](http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Nullable) to existing actions //// TODO null doesn't always mean the same thing as "default"; would action overloads be more appropriate for these cases? +nullable: +commit - https://msazure.visualstudio.com/One/_git/AD-AggregatorService-Workloads/commit/716416d651b4b0c835ae07840f1883532fff946c?refName=refs%2Fheads%2Fgdebruin%2Foptionalparameters&path=%2FWorkloads%2FMicrosoft.DirectoryServices%2Foverride%2Fschema-Review-beta.csdl&_a=compare +build - https://msazure.visualstudio.com/One/_build/results?buildId=88052971&view=logs&j=15dfcb1a-0989-5cf6-3160-3e181e44de87&t=44b6206c-e047-57cb-8bd1-54146504f81b + +optional parameter: + + @@ -444,18 +451,26 @@ GET /containers //// TODO check linting rules are ok with this guidance before shipping it - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? -//// TODO 5xx can be changed to 4xx + +Redirects: +2xx can be changed to 3xx //// TODO does everyone agree with this? +4xx can be changed to 3xx //// TODO does everyone agree with this? +5xx can be changed to 3xx //// TODO does everyone agree with this? + +Success: +we've decided in the past that we can't change 200 to 202 +//// TODO can any other changes be made? + +Error: +5xx can be changed to 4xx //// TODO does everyone agree with this? //// TODO cannot within the 4xx's //// can you change within 5xx's? -//// clients should never code against 400 specifically; in these branches, they should check for *any* 4xx -//// TODO do we want to require that innererrors remain the same? -//// TODO have success, redirect, and error categories -//// can't change 200 to 202 +//// TODO do we want to require that innererrors remain the same? //// TODO what about http status codes? the top-level error codes are expected to be the same as http status codes (is this guidance actually written?) -//// TODO beginning the use of redirects should be ok + - Introducing server-side pagination to existing collections //// TODO do we have an established pattern to introduce server-side pagination to existing collections? - Making significant changes to the performance of APIs such as increased latency, rate limits, or concurrency //// TODO related to the below comment, do we really want to specify what are breaking changes? aren't we really just creating a contract with client developers saying what are acceptable changes so that they can code defensively for them? i understand that it will limit us/make this document invalid if we ever need to do something that's *not* enumerated (like a header or something), but if we don't take a strong stance on this, then what's the point of the doc at all? From b3d96ab59ed1ed1f71344b7ef8619913fc965015 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Mon, 26 Feb 2024 07:58:10 -0800 Subject: [PATCH 32/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index fefa075b..6a9e9f00 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -426,6 +426,8 @@ GET /containers //// On the writing side of things, it seems like you could make `propName` into `foo`, except that now, previous requests would fail because they don't have `@odata.type` of `intermediate`. Changing it to `bar` would now prevent the `intermediate`s from being written entirely. //// Does anyone have examples of when change the `Type` attribute of a property is allowed? +- Change the `Type` attribute of a property unless the property is read-only, the new `Type` is a derived type of the original `Type`, and the property now only returns instances of the new `Type` or its derived types + - Removing or renaming APIs or API parameters - Adding a required request header - Adding EnumType members for nonevolvable enumerations From 0e7d9a4b7dea7dc317dbbb18a174ec1c4ed8bfb5 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Mon, 26 Feb 2024 08:01:11 -0800 Subject: [PATCH 33/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 6a9e9f00..c6f7e8d8 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -439,7 +439,8 @@ commit - https://msazure.visualstudio.com/One/_git/AD-AggregatorService-Workload build - https://msazure.visualstudio.com/One/_build/results?buildId=88052971&view=logs&j=15dfcb1a-0989-5cf6-3160-3e181e44de87&t=44b6206c-e047-57cb-8bd1-54146504f81b optional parameter: - +commit - https://msazure.visualstudio.com/One/_git/AD-AggregatorService-Workloads/commit/4715045eb3a38de22a1125c8f02d4d62ef495088?refName=refs%2Fheads%2Fgdebruin%2Foptionalparameters +build - https://msazure.visualstudio.com/One/_build/results?buildId=88114937&view=logs&j=15dfcb1a-0989-5cf6-3160-3e181e44de87&t=44b6206c-e047-57cb-8bd1-54146504f81b From e4293aaaf036931492cc2b1688db8d769f05e762 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 10:25:29 -0800 Subject: [PATCH 34/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index c6f7e8d8..4ac78f5e 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -395,11 +395,11 @@ option 2 - Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? //// TODO the compatible types are still lieklly to be breaking sdk changes for some languages - + - + - + @@ -407,9 +407,17 @@ option 2 - + + +PATCH /containers/{id} +{ + "propName": { + + } +} + GET /containers { @@ -427,6 +435,10 @@ GET /containers //// Does anyone have examples of when change the `Type` attribute of a property is allowed? - Change the `Type` attribute of a property unless the property is read-only, the new `Type` is a derived type of the original `Type`, and the property now only returns instances of the new `Type` or its derived types +//// TODO make the note about if intermediate is abstract; in this case, odata.type would be rqeuired to be returned still +//// TODO if there's a peer to bar, such as `baz` derives `intermediate`, it would still work to change it to `baz` +//// TODO are there edge cases where it's not "read-only" but actually "no creating" types? +//// TODO if there are no derived types of the newly specified type, nor of the existing specified type, then we just need duck typing - Removing or renaming APIs or API parameters - Adding a required request header From 9ce80792dfa9a04caa4df7842dcf8dbd65a5228b Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 10:36:16 -0800 Subject: [PATCH 35/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 4ac78f5e..21160760 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -440,6 +440,9 @@ GET /containers //// TODO are there edge cases where it's not "read-only" but actually "no creating" types? //// TODO if there are no derived types of the newly specified type, nor of the existing specified type, then we just need duck typing +//// TODO anything that doesn't change what's on the wire shouldn't be considered a breaking change; we should have documentation to the workload teams for how to maintain the on-the-wire representation for these cases, as well as what this will mean for their future maintainability +//// TODO we should also be careful for when URIs get broken; this needs to be accounted for + - Removing or renaming APIs or API parameters - Adding a required request header - Adding EnumType members for nonevolvable enumerations From 0c6a598de9ec4e92e55ac92fda54fe5b559b8951 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 10:41:13 -0800 Subject: [PATCH 36/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 21160760..a9f47922 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -446,7 +446,7 @@ GET /containers - Removing or renaming APIs or API parameters - Adding a required request header - Adding EnumType members for nonevolvable enumerations -- Adding Nullable="false" properties to existing types +- Adding Nullable="false" properties to existing types //// TODO what does this mean? - Adding a parameter not marked as [Nullable](http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Nullable) to existing actions //// TODO null doesn't always mean the same thing as "default"; would action overloads be more appropriate for these cases? nullable: @@ -467,6 +467,8 @@ build - https://msazure.visualstudio.com/One/_build/results?buildId=88114937&vie //// TODO can you overload actions? apparently not... //// TODO can we use the optionalparamter annotation? yes, we should suggest this //// TODO check linting rules are ok with this guidance before shipping it + +//// TODO actually take the optional parameter thing for action - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? From 1367316a77c4204650f94a1488164d0beef29ab7 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 10:49:32 -0800 Subject: [PATCH 37/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index a9f47922..e15bce78 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -474,9 +474,14 @@ build - https://msazure.visualstudio.com/One/_build/results?buildId=88114937&vie Redirects: 2xx can be changed to 3xx //// TODO does everyone agree with this? +//// we undersatnd existing apps might be impacted, but apps deved from this guindace going forward should expect this 4xx can be changed to 3xx //// TODO does everyone agree with this? 5xx can be changed to 3xx //// TODO does everyone agree with this? +3xx to 2xx? yes, we accept this; this is often a security concern, so we need to make this part of our idioms +3xx to 4xx? +3xx to 5xx? + Success: we've decided in the past that we can't change 200 to 202 //// TODO can any other changes be made? From 41882e0438fd25924b22e9ceed3072b7fbdf9d4e Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 10:53:34 -0800 Subject: [PATCH 38/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index e15bce78..2c822849 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -472,10 +472,14 @@ build - https://msazure.visualstudio.com/One/_build/results?buildId=88114937&vie - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? +no intentional changes from 3xx to 5xx + Redirects: 2xx can be changed to 3xx //// TODO does everyone agree with this? //// we undersatnd existing apps might be impacted, but apps deved from this guindace going forward should expect this +//// we are agreed 4xx can be changed to 3xx //// TODO does everyone agree with this? + 5xx can be changed to 3xx //// TODO does everyone agree with this? 3xx to 2xx? yes, we accept this; this is often a security concern, so we need to make this part of our idioms From ced379d98453755c2255b8adfcf49eb1f53f6573 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 10:55:04 -0800 Subject: [PATCH 39/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 2c822849..54ba908c 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -479,12 +479,16 @@ Redirects: //// we undersatnd existing apps might be impacted, but apps deved from this guindace going forward should expect this //// we are agreed 4xx can be changed to 3xx //// TODO does everyone agree with this? - +//// we are agreed 5xx can be changed to 3xx //// TODO does everyone agree with this? +//// we are agreed 3xx to 2xx? yes, we accept this; this is often a security concern, so we need to make this part of our idioms +//// we are agreed 3xx to 4xx? +//// we are agreed 3xx to 5xx? +//// we are agreed Success: we've decided in the past that we can't change 200 to 202 From 7c59146130d40f43ee12d63cdddcba26f8ccfc05 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 11:00:11 -0800 Subject: [PATCH 40/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 54ba908c..532acf32 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -492,6 +492,8 @@ Redirects: Success: we've decided in the past that we can't change 200 to 202 +204 to 200 should be allowed? maybe with a "feature flag" header? header isn't really different than `$select`s or the representation prefer header? + //// TODO can any other changes be made? Error: From 47f64df3b89bfe784c489f68d2142931ceb005e4 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 13:54:42 -0800 Subject: [PATCH 41/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 79 +++++++++++++++++++++++++++++++--------- 1 file changed, 62 insertions(+), 17 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 532acf32..ee80fe0b 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -395,11 +395,37 @@ option 2 - Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? //// TODO the compatible types are still lieklly to be breaking sdk changes for some languages - +### Case {1} + + + + + + + + + + + + + + + + + + + +#### Transition {a} - breaking? + + + - + + - + + + @@ -407,34 +433,53 @@ option 2 - + +**CHANGED** + +**/CHANGED** +GET /containers/{id} -PATCH /containers/{id} { "propName": { - + "prop1": "..." + // client doesn't know to not expect prop2 } } -GET /containers +#### Transition {b} - non-breaking + + + + + + + + + + + + + + + + + +**CHANGED** + + +**/CHANGED** + + +#### Transition {c} - -{ - "value": [ - { - "id": "{containerId1}", - "propName": { - } - } - ] -} //// I don't really see how you can change the type of `propName`. If you make it `foo`, then any `intermediate`s that are returned now have an odata.type that clients need to check. If you make it `bar`, then `intermediate`s can no longer be returned; further, if you only ever returned `bar`s before, they now wouldn't have the `@odata.type`. //// On the writing side of things, it seems like you could make `propName` into `foo`, except that now, previous requests would fail because they don't have `@odata.type` of `intermediate`. Changing it to `bar` would now prevent the `intermediate`s from being written entirely. //// Does anyone have examples of when change the `Type` attribute of a property is allowed? -- Change the `Type` attribute of a property unless the property is read-only, the new `Type` is a derived type of the original `Type`, and the property now only returns instances of the new `Type` or its derived types + //// TODO make the note about if intermediate is abstract; in this case, odata.type would be rqeuired to be returned still //// TODO if there's a peer to bar, such as `baz` derives `intermediate`, it would still work to change it to `baz` //// TODO are there edge cases where it's not "read-only" but actually "no creating" types? From 02957de65a31c1570c9d2aebe4ae62a1b46837bf Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:02:05 -0800 Subject: [PATCH 42/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 84 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index ee80fe0b..8b0abe01 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -415,7 +415,7 @@ option 2 -#### Transition {a} - breaking? +#### Transition {a} - TODO is this breaking? @@ -474,6 +474,87 @@ GET /containers/{id} #### Transition {c} - +### Case {2} + + + + + + + + + + + + + + + + + + + +#### Transition {a} - breaking + + + + + + + + + + + + + + + + + +**CHANGED** + +**/CHANGED** + + +PATCH /containers/{id} +{ + "prop2": "..." + // this fails now because there is no prop2 on base +} + +#### Transition {b} - TODO is this breaking? + + + + + + + + + + + + + + + + + +**CHANGED** + +**/CHANGED** + + +PATCH /containers/{id} +{ + "prop2": "..." + // prop3 is not provided; if prop3 is not required, this isn't a breaking change? +} + + + + //// I don't really see how you can change the type of `propName`. If you make it `foo`, then any `intermediate`s that are returned now have an odata.type that clients need to check. If you make it `bar`, then `intermediate`s can no longer be returned; further, if you only ever returned `bar`s before, they now wouldn't have the `@odata.type`. //// On the writing side of things, it seems like you could make `propName` into `foo`, except that now, previous requests would fail because they don't have `@odata.type` of `intermediate`. Changing it to `bar` would now prevent the `intermediate`s from being written entirely. @@ -483,6 +564,7 @@ GET /containers/{id} //// TODO make the note about if intermediate is abstract; in this case, odata.type would be rqeuired to be returned still //// TODO if there's a peer to bar, such as `baz` derives `intermediate`, it would still work to change it to `baz` //// TODO are there edge cases where it's not "read-only" but actually "no creating" types? + //// TODO if there are no derived types of the newly specified type, nor of the existing specified type, then we just need duck typing //// TODO anything that doesn't change what's on the wire shouldn't be considered a breaking change; we should have documentation to the workload teams for how to maintain the on-the-wire representation for these cases, as well as what this will mean for their future maintainability From e09742493cd401190b3455b2524f16a704c973c4 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:03:33 -0800 Subject: [PATCH 43/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 8b0abe01..e0123e5e 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -472,8 +472,6 @@ GET /containers/{id} **/CHANGED** -#### Transition {c} - - ### Case {2} From 79b07bd29cad0351454ad13948ff0993a48f0441 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 14:07:32 -0800 Subject: [PATCH 44/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 57 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index e0123e5e..54bec246 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -550,16 +550,65 @@ PATCH /containers/{id} // prop3 is not provided; if prop3 is not required, this isn't a breaking change? } +### Case {3} + + + + + + + + + + + + + + + + + + +#### Transition {a} - non-breaking + + + + + + + + + + + + + + + + + +**CHANGED** + + +**/CHANGED** + + +GET /containers/{id} + +{ + "propName": { + "@odata.type": "#self.derived", // the @odata.type is still required since existing clients are likely looking for it from the previous version where they could get other types derived from intermediate + "prop1": "...", + "prop2": "...", + "prop3": "..." + } +} -//// I don't really see how you can change the type of `propName`. If you make it `foo`, then any `intermediate`s that are returned now have an odata.type that clients need to check. If you make it `bar`, then `intermediate`s can no longer be returned; further, if you only ever returned `bar`s before, they now wouldn't have the `@odata.type`. -//// On the writing side of things, it seems like you could make `propName` into `foo`, except that now, previous requests would fail because they don't have `@odata.type` of `intermediate`. Changing it to `bar` would now prevent the `intermediate`s from being written entirely. -//// Does anyone have examples of when change the `Type` attribute of a property is allowed? -//// TODO make the note about if intermediate is abstract; in this case, odata.type would be rqeuired to be returned still //// TODO if there's a peer to bar, such as `baz` derives `intermediate`, it would still work to change it to `baz` //// TODO are there edge cases where it's not "read-only" but actually "no creating" types? From a741c381ba3eb78c20b99e992186202a44da326f Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 15:15:40 -0800 Subject: [PATCH 45/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 54bec246..70092ff2 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -567,7 +567,7 @@ PATCH /containers/{id} - + #### Transition {a} - non-breaking @@ -609,7 +609,6 @@ GET /containers/{id} -//// TODO if there's a peer to bar, such as `baz` derives `intermediate`, it would still work to change it to `baz` //// TODO are there edge cases where it's not "read-only" but actually "no creating" types? //// TODO if there are no derived types of the newly specified type, nor of the existing specified type, then we just need duck typing From f9f18d65f878ff4065815c38aba465760f525ccd Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 15:17:03 -0800 Subject: [PATCH 46/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 70092ff2..7881f176 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -608,9 +608,6 @@ GET /containers/{id} - -//// TODO are there edge cases where it's not "read-only" but actually "no creating" types? - //// TODO if there are no derived types of the newly specified type, nor of the existing specified type, then we just need duck typing //// TODO anything that doesn't change what's on the wire shouldn't be considered a breaking change; we should have documentation to the workload teams for how to maintain the on-the-wire representation for these cases, as well as what this will mean for their future maintainability From c3221804c10d1bd131caec4737c4edebf9cde48f Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 15:42:16 -0800 Subject: [PATCH 47/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 78 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 4 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 7881f176..1b050504 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -392,8 +392,13 @@ option 2 - Removing control information from a response payload if the current behavior is that the control information is always present in the response payload - Adding a property that is required for the creation of the type it is defined on - Changing the URL or fundamental request/response associated with a resource -- Removing, renaming, or changing an incompatible type of a declared property //// TODO should we make clear what incompatible types are add making "compatible" type changes to the non-breaking list? -//// TODO the compatible types are still lieklly to be breaking sdk changes for some languages +- Removing, renaming, or changing an incompatible type of a declared property + +**TODO start here** +//// TODO i would like to propose that we don't allow any changes to the `Type` attribute +//// TODO is this a potential guiding principle? anything that doesn't change what's on the wire shouldn't be considered a breaking change; we should have documentation to the workload teams for how to maintain the on-the-wire representation for these cases, as well as what this will mean for their future maintainability + + ### Case {1} @@ -605,13 +610,78 @@ GET /containers/{id} } } +### Case {4} + + + + + + + + + + + + + -//// TODO if there are no derived types of the newly specified type, nor of the existing specified type, then we just need duck typing +#### Transition {a} - non-breaking + + + + + + + + + + + + + + +**CHANGED** + +**/CHANGED** + + +**HOWEVER**, this does break (maybe? depends on our answer to other questions) if we make a *future* change: + + + + + +**CHANGED** + + + +**/CHANGED** + + + + + + + + + + + + +GET /containers/{id} + +{ + "propName": { + "@odata.type": "$self.fooDerived", + "prop1": "..." + // client doesn't know to not expect prop2; also, for the write case, a client would have needed to be updated to try *writing* a fooDerived, so that client should be expected to realize that propName is not of type foo anymore + } +} -//// TODO anything that doesn't change what's on the wire shouldn't be considered a breaking change; we should have documentation to the workload teams for how to maintain the on-the-wire representation for these cases, as well as what this will mean for their future maintainability //// TODO we should also be careful for when URIs get broken; this needs to be accounted for +//// TODO the compatible types are still lieklly to be breaking sdk changes for some languages - Removing or renaming APIs or API parameters - Adding a required request header From 60b21fe1ae0a3155e5756ad1c5c0343696204657 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 15:43:33 -0800 Subject: [PATCH 48/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 1b050504..eeee7771 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -686,7 +686,7 @@ GET /containers/{id} - Removing or renaming APIs or API parameters - Adding a required request header - Adding EnumType members for nonevolvable enumerations -- Adding Nullable="false" properties to existing types //// TODO what does this mean? +- Adding Nullable="false" properties to existing types //// TODO this should be "adding properties that have default values" (whether or not the `DefaultValue` attribute is used) - Adding a parameter not marked as [Nullable](http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Nullable) to existing actions //// TODO null doesn't always mean the same thing as "default"; would action overloads be more appropriate for these cases? nullable: From 6e64472200471be560eb2f3616a32077a294dc71 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 15:46:13 -0800 Subject: [PATCH 49/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 23 +---------------------- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index eeee7771..184d6a5d 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -687,28 +687,7 @@ GET /containers/{id} - Adding a required request header - Adding EnumType members for nonevolvable enumerations - Adding Nullable="false" properties to existing types //// TODO this should be "adding properties that have default values" (whether or not the `DefaultValue` attribute is used) -- Adding a parameter not marked as [Nullable](http://docs.oasis-open.org/odata/odata-csdl-xml/v4.01/odata-csdl-xml-v4.01.html#sec_Nullable) to existing actions //// TODO null doesn't always mean the same thing as "default"; would action overloads be more appropriate for these cases? - -nullable: -commit - https://msazure.visualstudio.com/One/_git/AD-AggregatorService-Workloads/commit/716416d651b4b0c835ae07840f1883532fff946c?refName=refs%2Fheads%2Fgdebruin%2Foptionalparameters&path=%2FWorkloads%2FMicrosoft.DirectoryServices%2Foverride%2Fschema-Review-beta.csdl&_a=compare -build - https://msazure.visualstudio.com/One/_build/results?buildId=88052971&view=logs&j=15dfcb1a-0989-5cf6-3160-3e181e44de87&t=44b6206c-e047-57cb-8bd1-54146504f81b - -optional parameter: -commit - https://msazure.visualstudio.com/One/_git/AD-AggregatorService-Workloads/commit/4715045eb3a38de22a1125c8f02d4d62ef495088?refName=refs%2Fheads%2Fgdebruin%2Foptionalparameters -build - https://msazure.visualstudio.com/One/_build/results?buildId=88114937&view=logs&j=15dfcb1a-0989-5cf6-3160-3e181e44de87&t=44b6206c-e047-57cb-8bd1-54146504f81b - - - - - - - - -//// TODO can you overload actions? apparently not... -//// TODO can we use the optionalparamter annotation? yes, we should suggest this -//// TODO check linting rules are ok with this guidance before shipping it - -//// TODO actually take the optional parameter thing for action +- Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing action //// TODO this is currently broken in the linting rules; file a bug and make a note that this should be expected: https://msazure.visualstudio.com/One/_git/AD-AggregatorService-Workloads/pullrequest/9577509?_a=files - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? From 778df3478b872dc487cf107921a60b957b590ae2 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 28 Feb 2024 15:48:09 -0800 Subject: [PATCH 50/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 184d6a5d..807b1bc0 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -381,6 +381,8 @@ option 2 - Adding a new base type to an existing type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing type into the base type - Adding a new derived type to an existing type //// TODO we should ghave guidance for workloads + clients regardless; evolvable enums but for derived types? maybe the guidnace should be that it shuold be treated as a breaking change from a "customer communication" p[oint of view (like a blog post or something); you need to follow up with others to really nail this down, it's not just a one-liner; TODO follow up if this is an SDK break https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707412726923?context=%7B%22contextType%22%3A%22chat%22%7D - Adding a new type in the inheritance hierarchy between an existing type and its current base type provided that no property `Type` attributes are changed that currently reference the existing type; this includes moving properties from the existing child type into the new base type +- Updating an HTTP status code from 3xx to anything else +- Updating an HTTP status from anything to 3xx //// TODO ahve a breaking change article that has samples, and in this doc have hihg level categories of brekaing changes @@ -691,24 +693,6 @@ GET /containers/{id} - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? -no intentional changes from 3xx to 5xx - -Redirects: -2xx can be changed to 3xx //// TODO does everyone agree with this? -//// we undersatnd existing apps might be impacted, but apps deved from this guindace going forward should expect this -//// we are agreed -4xx can be changed to 3xx //// TODO does everyone agree with this? -//// we are agreed -5xx can be changed to 3xx //// TODO does everyone agree with this? -//// we are agreed - -3xx to 2xx? yes, we accept this; this is often a security concern, so we need to make this part of our idioms -//// we are agreed -3xx to 4xx? -//// we are agreed -3xx to 5xx? -//// we are agreed - Success: we've decided in the past that we can't change 200 to 202 204 to 200 should be allowed? maybe with a "feature flag" header? header isn't really different than `$select`s or the representation prefer header? From ba35037c8a2a82ec09c516fa917c98ff807f49d1 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Fri, 8 Mar 2024 08:01:59 -0800 Subject: [PATCH 51/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 807b1bc0..d587c3ce 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -349,7 +349,7 @@ In general, making all but additive changes to the API contract for existing ele //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support#api-contract-and-nonbackward-compatible-changes //// TODO sync this with https://learn.microsoft.com/en-us/graph/versioning-and-support //// TODO update verbiage of sdk vs api versions: https://teams.microsoft.com/l/message/19:a87c7e39-080d-45df-abfa-956c25d852c7_c3e0b685-1b22-4bd3-a5f2-ad4f17c5a30d@unq.gbl.spaces/1707321601369?context=%7B%22contextType%22%3A%22chat%22%7D -//// TODO we should further discuss model annotations? we will not document these as breaks today; we will kick this down the road for when we are using typespec and the annotations that graph is publishing are actually accurate +//// TODO we should further discuss model annotations? we will not document these as breaks today; we will kick this down the road for when we are using typespec and the annotations that graph is publishing are actually accurate; when we re-evaluate, we should go through the kinds of annotations that are used and make judgment calls on each; consider sdk generation, client code generation, docs generation, and ags features that leverage annotations /*TODO sdk breaks differentiate between sdk vs rest breaking changes; also differentiate if clients need to update the sdk for it to be a break: @@ -713,6 +713,7 @@ Error: - Making significant changes to the performance of APIs such as increased latency, rate limits, or concurrency //// TODO related to the below comment, do we really want to specify what are breaking changes? aren't we really just creating a contract with client developers saying what are acceptable changes so that they can code defensively for them? i understand that it will limit us/make this document invalid if we ever need to do something that's *not* enumerated (like a header or something), but if we don't take a strong stance on this, then what's the point of the doc at all? //// TODO make clear any of the "add stuff to the type hierarchy" changes that *are* breaking? e.g. adding an intermediate type and changing a `Type` attribute (breaks clients that are currently using `@odata.type`), adding an intermediate type and moving a property from the base type to the new type, etc.? +//// TODO how do we introduce new breaking change guidance? The applicable changes described in the [Model Versioning of the OData V4.01 spec](https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_ModelVersioning) SHOULD be considered part of the minimum bar that all services MUST consider a breaking change. From 1c948bc00e15a60a2c376348ad22ef3c869ee358 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 13 Mar 2024 10:54:08 -0700 Subject: [PATCH 52/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index d587c3ce..a2c01527 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -693,14 +693,17 @@ GET /containers/{id} - Adding a parameter not marked as [Optional](https://github.com/oasis-tcs/odata-vocabularies/blob/main/vocabularies/Org.OData.Core.V1.md#OptionalParameter) to an existing function - Changing top-level error codes //// TODO is this really a rule? to what extent do we hold ourselves to this standard? +//// TODO write out the guidnace that 3xx's are allowed Success: -we've decided in the past that we can't change 200 to 202 +we've decided in the past that we can't change 200 to 202 //// we should keep this guidance 204 to 200 should be allowed? maybe with a "feature flag" header? header isn't really different than `$select`s or the representation prefer header? //// TODO can any other changes be made? Error: -5xx can be changed to 4xx //// TODO does everyone agree with this? +5xx can be changed to 4xx //// TODO does everyone agree with this? yes, everyone agress + + //// TODO cannot within the 4xx's //// can you change within 5xx's? From 728ebfda2bd252d231b03601af0d40d03417a181 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Wed, 13 Mar 2024 11:02:12 -0700 Subject: [PATCH 53/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index a2c01527..6d24c664 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -704,7 +704,7 @@ Error: 5xx can be changed to 4xx //// TODO does everyone agree with this? yes, everyone agress -//// TODO cannot within the 4xx's +//// TODO cannot within the 4xx's //// getting lots of feedback that we *shouldn't* allow this, some feedback about specific cases where it makes sense; go through the status codes offline and bring back a proposal //// can you change within 5xx's? From f812369e9837df15eda5416c564311af815c8588 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Thu, 25 Apr 2024 12:27:52 -0700 Subject: [PATCH 54/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 1 + 1 file changed, 1 insertion(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 6d24c664..6bad8c7b 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -704,6 +704,7 @@ Error: 5xx can be changed to 4xx //// TODO does everyone agree with this? yes, everyone agress +//// TODO during a discussion about allowing a change from 201 to 409 (because duplicates were being created), there was a lot of sentiment that "fixing" things shouldn't be considered a break; it was also stated that going from 201 to 409 *is* a break if it wasn't part of the original design of the API (i.e. duplicates were intended initially, but now they are not is a semantic change taht *should* be considered breaking and should follow a proper deprecation process); it *might* be worth writing down the specific case where 201 to 409 is "not a break" (the case where allowing duplicates was not part of the original design), but this would just be a subset of "fixing things shouldn't be considered a break", so if that is our guidance, then we shouldn't need to write down the specifics of 201 to 409 //// TODO cannot within the 4xx's //// getting lots of feedback that we *shouldn't* allow this, some feedback about specific cases where it makes sense; go through the status codes offline and bring back a proposal //// can you change within 5xx's? From d3f026fd02bd3369fd97fcb4d88547e3b16be803 Mon Sep 17 00:00:00 2001 From: Garrett DeBruin <16618938+corranrogue9@users.noreply.github.com> Date: Sat, 27 Apr 2024 07:17:50 -0700 Subject: [PATCH 55/55] Update GuidelinesGraph.md --- graph/GuidelinesGraph.md | 1 + 1 file changed, 1 insertion(+) diff --git a/graph/GuidelinesGraph.md b/graph/GuidelinesGraph.md index 6bad8c7b..4547123f 100644 --- a/graph/GuidelinesGraph.md +++ b/graph/GuidelinesGraph.md @@ -358,6 +358,7 @@ removing open type and schematizing: the schematized property will no longer be sdks can have issues with expanding type hierarchies adding a new base type (regardless of if referencing properties are changed) adding an intermediate type +make sure to document this explicit customer communication about sdk breaks: https://teams.microsoft.com/l/message/19:meeting_ZmM3YTQ5ODEtYzNkZC00M2ViLTk1YTAtODYyMjVhOTRkNjQz@thread.v2/1714142731942?context=%7B%22contextType%22%3A%22chat%22%7D */ option 1