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

Renamed TranslationFilter and DocumentFilter to Get[Translation/Document]StatusesOptions #23336

Merged
merged 5 commits into from
Aug 19, 2021

Conversation

AhmedLeithy
Copy link
Member

TranslationFilter --> GetTranslationStatusesOptions
DocumentFilter --> GetDocumentStatusesOptions
as per #23088

Should we also change TranslationFilterOrder and TranslationFilterProperty? (DocumentFilter.. as well)

TranslationFilterOrder --> GetTranslationStatusesOptionsOrder seems a bit long and confusing in my opinion, but leaving them as is might also be a bit confusing. What do you think?

@maririos
Copy link
Member

Should we also change TranslationFilterOrder and TranslationFilterProperty? (DocumentFilter.. as well)
TranslationFilterOrder --> GetTranslationStatusesOptionsOrder seems a bit long and confusing in my opinion, but leaving them as is might also be a bit confusing. What do you think?

I agree with your decision. Let's leave those types as is

@AhmedLeithy
Copy link
Member Author

AhmedLeithy commented Aug 18, 2021

Without the customized model, the generated filter is already internal. So I removed DocumentStatusInternal.cs all together. The only difference this cause is that the model is generated in the namespace ...Document.Models, but I don't think that would be an issue.

TranslationFilter --> GetTranslationStatusesOptions
DocumentFilter --> GetDocumentStatusesOptions
Copy link
Member

@jsquire jsquire left a comment

Choose a reason for hiding this comment

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

Please rebase on main before merging. There are a lot of unrelated changes riding along that we should remove.

TranslationFilter --> GetTranslationStatusesOptions
DocumentFilter --> GetDocumentStatusesOptions
@AhmedLeithy
Copy link
Member Author

Please rebase on main before merging. There are a lot of unrelated changes riding along that we should remove.

Done! Apologies.

@maririos maririos merged commit 878e25f into Azure:main Aug 19, 2021
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-net that referenced this pull request Apr 12, 2023
[Hub Generated] Review request for Microsoft.Insights to add version preview/2023-03-01-preview (Azure#23336)

* Adds base for updating preview from version 2018-09-01-preview/examples to version 2023-03-01-preview

* Updates readme

* Updates API version in new specs and examples

* Add metricsbatch swagger

* Convert dataplane swagger to openapi 2.0

* Fix response type

* Update response schema

* Lint fixes

* Unexpected token fix

* Prettier

* PR comments

* Comments

* Add resourceid to custom words

* More PR comments

* Fix model

* PR comment

* Small fix to example request

* Address feedback from review

* Update custom words

* Small fix

* Change descriptions for localizable string

* Add missing description

* More feedback changes

* Small grammar fixes

* Update endpoint name

* Update interval description

---------

Co-authored-by: gulopesd <73562152+gulopesd@users.noreply.github.com>
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.

3 participants