Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge enums #495

Open
wants to merge 6 commits into
base: vNext
Choose a base branch
from
Open

Merge enums #495

wants to merge 6 commits into from

Conversation

XVincentX
Copy link
Member

Closes #494

@XVincentX XVincentX marked this pull request as ready for review August 28, 2023 19:36
@XVincentX XVincentX requested review from dkershaw10, corranrogue9 and a team August 29, 2023 19:25
@OlgaPodo OlgaPodo added Graph Guidelines This should be reviewed by Microsoft Grap team. Microsoft Graph This should be reviewed by Microsoft Grap team. labels Aug 30, 2023

## Problem

The API Design requires that a specific property of a data structure has a finite and limited set of values, and such limitation needs to be clearly communicated to the user and give the developer the same structure
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


## Solution

In OData, Enums represent a subset of the nominal type they rely on, and are especially useful in cases where certain properties have predefined, limited options.

Copy link
Collaborator

@OlgaPodo OlgaPodo Sep 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enum solution must be based on the evolvable enum. Not evolvable enums are exceptions. Could you please merge all template sections together? it doesn't make sense to have the same section in one document twice.


- Our SDK generators will translate the enum to the best representation of the target programming language, resulting in a better developer experience and free client side validation

#### Cons
### Cons

- Adding a new value requires to go through a (generally fast) API Review
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our internal process shouldn't impact API design considerations especially as they published for external users.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it is a clear side effect which is annoying for a lot of workload owners. I can remove it if you want, but we're missing a particular part that affect us. Your call

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this. We might offer some process guidance in the future, but I don't think it's our area of expertise now or something we are working on.


## Issues and considerations

### Pros

- Our SDK generators will translate the enum to the best representation of the target programming language, resulting in a better developer experience and free client side validation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the pros for Enums is that clients can write a processing logic based on the enum values.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that true for any property? Clients write processing logic based on values - not too sure what is different here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Graph Guidelines This should be reviewed by Microsoft Grap team. Microsoft Graph This should be reviewed by Microsoft Grap team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix content table in the main Graph guidelines. merger enum and evolvable enum patterns
2 participants