add guidance for transitioning from a structural collection to a navigation collection #551
Conversation
graph/articles/collections.md
Outdated
| </Key> | ||
| <Property Name="id" Type="Edm.String" Nullable="false" /> | ||
| <Property Name="bars" Type="Collection(self.bar)" /> | ||
| + <NavigationProperty Name="barsAsEntities" Type="Collection(self.barAsEntity)" ContainsTarget="true" /> |
There was a problem hiding this comment.
We wouldn't really want them to name barsAsEntities, would we? I feel like Entities isn't a concept the api consumer necessarily needs to understand.
There was a problem hiding this comment.
I'm open to other naming conventions, but I do think a naming convention needs to be established for this. If we take your other suggestions and drive workloads to deprecate the old property, then this new property is temporary and can be moved to use the old name if desired. I don't personally have an issue with this name because a client that is wanting to update this collection at least needs to know enough about OData to understand that this is the property that they need to use (instead of the old property). I'm normally opposed to hungarian notation, but in this instance, the difference between the two properties is literally the type, and so the naming needs to reflect that.
graph/articles/collections.md
Outdated
| ``` | ||
| The expectation is that `bars` and `barsAsEntities` are treated as two "views" into the same data. | ||
| To meet this expectation, workloads must: | ||
| 1. Keep the properties consistent between `bar` and `barAsEntity`. |
There was a problem hiding this comment.
Do we really want to keep maintaining bar, or would it be better to deprecate bar and only add new properties to barAsEntity?
There was a problem hiding this comment.
At the very least, this section needs to be honored by workloads for the period that both properties exist. I think we are agreed on that?
I personally don't feel strongly about driving workloads to deprecate the old property. I think that their own supportability overhead will be a forcing function for them, and if they don't have significant overhead, I don't see a problem with having both properties.
Maybe I should just add a sentence that says that the old property should be marked deprecated (and leave out whether it ever gets removed)?
graph/articles/collections.md
Outdated
| <PropertyRef Name="id" /> | ||
| </Key> | ||
| <Property Name="id" Type="Edm.String" Nullable="false" /> | ||
| <Property Name="bars" Type="Collection(self.bar)" /> |
There was a problem hiding this comment.
In this case, I would suggest deprecating bars in favor of barsAsEntities.
There was a problem hiding this comment.
I think the other comment has more context, but I'll leave this open for now pending that outcome.
OlgaPodo
added
Graph Guidelines
Co-authored-by: Michael Pizzo <mikep@microsoft.com>
Co-authored-by: Michael Pizzo <mikep@microsoft.com>
No description provided.