DO NOT MERGE - SMOKE TEST #2004
Conversation
baywet
changed the title
|
@andrueastman I triggered that generation for sanity to make sure the few last minute changes in kiota didn't have unanticipated side effect. Enum options now get their value escaped when they didn't. I think we should trigger a generation for Go for sanity and double check we don't get unwanted values escaped. namespace Microsoft.Graph.Models {
public enum ExternalEmailOtpState {
[EnumMember(Value = "default")]
- Default,
+ @Default,The following models are removed, to me it looks like they are not used anywhere and they got rightfully trimmed, but I'd like a confirmation:
|
The enum renaming really is a regression after u
I'm not too sure about theses getting rightfully trimmed. The real reasons is that the discriminator for the |
| @@ -40,11 +40,7 @@ public class WorkflowExecutionTrigger : IAdditionalDataHolder, IBackedModel, IPa | |||
| /// <param name="parseNode">The parse node to use to read the discriminator value and create the object</param> | |||
| public static WorkflowExecutionTrigger CreateFromDiscriminatorValue(IParseNode parseNode) { | |||
| _ = parseNode ?? throw new ArgumentNullException(nameof(parseNode)); | |||
| var mappingValue = parseNode.GetChildNode("@odata.type")?.GetStringValue(); | |||
| return mappingValue switch { | |||
| "#microsoft.graph.identityGovernance.timeBasedAttributeTrigger" => new TimeBasedAttributeTrigger(), | |||
There was a problem hiding this comment.
Removing this mapping causes the dropping of the two types. cc @baywet
There was a problem hiding this comment.
The only reference to that type is in triggerAndScopeBasedConditions, which is not directly referenced but derives from workflowExecutionConditions.
There's no way of directly accessing WorkflowExecutionTrigger and I'd in fact argue that this type itself should have been trimmed too.
There was a problem hiding this comment.
Just to confirm here. What were are saying is that since this is a derived type of a property from a derived type of another property, we would classify that as being to far off to be included?
There was a problem hiding this comment.
I'm guessing that if you get a workflowBase, for which the executionConditions property value is in fact a triggerAndScopeBasedConditions (derived from the declared type) of which the trigger property is in fact a timeBasedAttributeTrigger (derived from the declared type) you'd need it. Which makes this a breaking change.
I'm guessing this is cause by that change (no condition previously), I'm just worried that removing the condition would end up pulling too many types.
https://github.com/microsoft/kiota/blob/06e9deac11c68ff683ddb3f57504e7f655094829/src/Kiota.Builder/KiotaBuilder.cs#L1763
Let me look into it further
|
starting a new generation. |
This pull request was automatically created by Azure Pipelines. Important Check for unexpected deletions or changes in this PR.
Microsoft Reviewers: Open in CodeFlow