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

Update generated code. #247

Merged
merged 3 commits into from
Feb 4, 2023
Merged

Update generated code. #247

merged 3 commits into from
Feb 4, 2023

Conversation

calebkiage
Copy link
Contributor

@calebkiage calebkiage marked this pull request as ready for review January 31, 2023 18:51
Base automatically changed from chore/enable-nullable-reference-types to main February 1, 2023 05:32
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

It's a great set of changes, the CLI is most likely going to be much more resiliant. I think the only pattern that we'd probably change is the following

var response = await RequestAdapter.SendPrimitiveAsync<Stream>(requestInfo, errorMapping: errorMapping, cancellationToken: cancellationToken) ?? Stream.Null;
response = (response is not null) ? await outputFilter.FilterOutputAsync(response, query, cancellationToken) : response;

response at the first line will never be null, so the second line will always apply the filter. We might want to compare the reference to stream.null instead or always call the filtering logic if it handles empty streams gracefully

@calebkiage
Copy link
Contributor Author

calebkiage commented Feb 1, 2023

Hey Vincent, I can add a check against Stream.Null

baywet
baywet previously approved these changes Feb 3, 2023
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

LGTM. You're probably impacted by microsoft/kiota#2234 but this is probably an edge case (excel endpoints only) that'll resolve itself whenever it's fixed

@calebkiage
Copy link
Contributor Author

I wonder if we should wait on microsoft/kiota#2251

@calebkiage calebkiage merged commit 2312dd3 into main Feb 4, 2023
@calebkiage calebkiage deleted the chore/nrt-code-generation branch February 4, 2023 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants