Skip to content

Generated v1.0 models and request builders using Typewriter - #845

Merged
Ramses Sanchez-Hernandez (ramsessanchez) merged 8 commits into
devfrom
v1.0/pipelinebuild/54101
Aug 11, 2021
Merged

Generated v1.0 models and request builders using Typewriter#845
Ramses Sanchez-Hernandez (ramsessanchez) merged 8 commits into
devfrom
v1.0/pipelinebuild/54101

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

This pull request was automatically created by the GitHub Action, create pull request.

The commit hash is 1932dd7.

Important Check for unexpected deletions or changes in this PR.

@baywet Vincent Biret (baywet) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

4-2-0_to_java.zip
Mustafa Zengin (@zengin) Andrew Omondi (@andrueastman) Ramses Sanchez-Hernandez (@ramsessanchez) this update technically introduced a breaking change: the targetApps actions now bind to targetedManagedAppProtection which inherits managedAppProtection instead of managedAppProtection.
Although this would only break people using the request builders directly and not through the fluent API, it's a similar case to the Teams API replacing identitySet by chat message identity set a couple of weeks ago which "forced" us to major rev. See #818
I wanted to get your opinion on this one?

CC DeVere Dyett (@ddyett)

@baywet Vincent Biret (baywet) added this to the 4.3.0 milestone Aug 10, 2021
@baywet

Copy link
Copy Markdown
Member

I took the time to chat about that to DeVere Dyett (@ddyett) and his instructions are to major rev same thing for dotnet Andrew Omondi (@andrueastman)

@andrueastman

Copy link
Copy Markdown
Contributor

Thanks for the heads up Vincent Biret (@baywet)

@baywet

Copy link
Copy Markdown
Member

Ramses Sanchez-Hernandez (@ramsessanchez) would you mind bumping the major version for this one please?

@andrueastman

Copy link
Copy Markdown
Contributor

Vincent Biret (@baywet), Ramses Sanchez-Hernandez (@ramsessanchez)
I just noticed that the diff for this PR is may not be consistent with the metadata change.
According to the commit today on the metadata here the targetApps is a newly added action (This is consistent with the .net generation).
Any chance this is what we expect?

@baywet

Copy link
Copy Markdown
Member

Thanks for the additional information Andrew Omondi (@andrueastman)

If we compare the previous full metadata file with the current one we can see that 2 things happened:

  • a new targetApps action on bound to targetedManagedAppProtection was added
  • a new targetedManagedAppGroupType parameter was added to the targetApps action bound to targetedManagedAppConfiguration

Because targetedManagedAppProtection inherits from managedAppProtection and managedAppProtection already had an action on it, we were generating the request builders and parameter sets for targetedManagedAppProtection as well. (that might only have worked if people were using OData Cast, if the API supports OData cast)
Now that a new action is defined with more precision on the child type, the generation process updated the generated request builder and parameter set.

You can have a look at the pkgdiff, specially at the following files

  • diffs/com/microsoft/graph/requests/TargetedManagedAppProtectionRequestBuilder.class-diff.html
  • diffs/com/microsoft/graph/requests/IosManagedAppProtectionRequestBuilder.class-diff.html
  • (and others)

Now, the chances of anyone hitting these breaking changes are low, but it's still technically a breaking change for Java. And it might be for dotnet, depending on whether or not dotnet generates actions request builders inherited from parent types. Hence the major bump on Java/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking the time to explain this Vincent Biret (@baywet).
I don't think we will need to bump up the major version for dotnet this time as we do not generate action request builders inherited from parent types at moment.
In the dotnet case the change simply shows up as a new method in the "child" request builder class.

@baywet

Copy link
Copy Markdown
Member

Ramses Sanchez-Hernandez (@ramsessanchez) we should be good to merge and release whenever you have time :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants