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

Make AmqpMessageId and AmqpAddress a readonly struct #31345

Merged
merged 4 commits into from
Sep 22, 2022

Conversation

danielmarbach
Copy link
Contributor

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

@ghost ghost added Azure.Core customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Sep 22, 2022
@ghost
Copy link

ghost commented Sep 22, 2022

Thank you for your contribution @danielmarbach! We will review the pull request and get back to you soon.

@ghost ghost added the Community Contribution Community members are working on the issue label Sep 22, 2022
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.

LGTM, but I'd like @JoshLove-msft to offer thoughts, just in case there's some scenario that I'm not thinking of.

@jsquire
Copy link
Member

jsquire commented Sep 22, 2022

Looks like CI is failing on Analyze due to needing to export the API.

@danielmarbach
Copy link
Contributor Author

Will fix the API export. Sorry that is something I never remember to run :(

@danielmarbach danielmarbach changed the title Make AmqpMessageId a readonly struct Make AmqpMessageId and AmqpAddress a readonly struct Sep 22, 2022
@jsquire
Copy link
Member

jsquire commented Sep 22, 2022

Will fix the API export. Sorry that is something I never remember to run :(

You're not alone there. 😄

Copy link
Member

@JoshLove-msft JoshLove-msft left a comment

Choose a reason for hiding this comment

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

LGTM!

@jsquire jsquire merged commit 0a24261 into Azure:main Sep 22, 2022
@danielmarbach danielmarbach deleted the readonly-struct branch September 22, 2022 17:51
sofiar-msft pushed a commit to sofiar-msft/azure-sdk-for-net that referenced this pull request Dec 7, 2022
* Make AmqpMessageId a readonly struct

* Make AmqpAddress a readonly struct

* Approve API changes

* Change log
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants