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

ObsoleteAttribute on method parameters #5228

Merged
merged 3 commits into from
Aug 26, 2024

Conversation

trejjam
Copy link
Contributor

@trejjam trejjam commented Aug 23, 2024

Hi,
using the Apple Store Connect API I found out that Kiota generates ObsoleteAttribute on methor parameters. That is, unfortunately, invalid code see ObsoleteAttribute.

This PR should fix it

@trejjam trejjam requested a review from a team as a code owner August 23, 2024 21:59
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.

Thank you for the contribution.
While the addition of the unit test is a great way to avoid regressions over time, I'm not sure adding an integration test like it is defined now adds a lot of value. Can you provide more context here please?

Can you also add a changelog entry please? (Unreleased, changed)

@trejjam
Copy link
Contributor Author

trejjam commented Aug 26, 2024

The integration test uses a snippet from the Apple API I used for testing. It's OK for me to remove it

@trejjam trejjam force-pushed the feature/obsolete-parameters branch from 3171a1b to 5feda01 Compare August 26, 2024 11:19
@baywet
Copy link
Member

baywet commented Aug 26, 2024

The integration test uses a snippet from the Apple API I used for testing. It's OK for me to remove it

I'd like @andrueastman opinion on this one, but as it is, I think we should remove it.

@trejjam trejjam force-pushed the feature/obsolete-parameters branch from 5feda01 to 5e547ef Compare August 26, 2024 11:57
@trejjam
Copy link
Contributor Author

trejjam commented Aug 26, 2024

Done

@trejjam trejjam requested a review from baywet August 26, 2024 11:58
baywet
baywet previously approved these changes Aug 26, 2024
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.

Thank you for making the changes!

@baywet baywet enabled auto-merge (squash) August 26, 2024 12:01
auto-merge was automatically disabled August 26, 2024 12:08

Head branch was pushed to by a user without write access

@trejjam trejjam force-pushed the feature/obsolete-parameters branch from 5e547ef to 88aedfd Compare August 26, 2024 12:08
@trejjam
Copy link
Contributor Author

trejjam commented Aug 26, 2024

There was a merge conflict in the CHANGELOG.md

@baywet baywet enabled auto-merge (squash) August 26, 2024 12:13
@baywet baywet merged commit 022e3e9 into microsoft:main Aug 26, 2024
219 of 226 checks passed
@trejjam trejjam deleted the feature/obsolete-parameters branch August 26, 2024 12:47
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