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

azcontainerregistry: move method to function #23270

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

stevekuznetsov
Copy link
Contributor

The Go documentation strongly discourages structs from having methods with both pointer recievers and non-pointer recievers. For the auth policy, getChallengeRequest presumably did not have a pointer reciever since it did not make use of the struct at all. We can simply move this to a function.

@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Aug 2, 2024
Copy link

github-actions bot commented Aug 2, 2024

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

Copy link
Member

@chlowell chlowell left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this!

The Go documentation strongly discourages structs from having methods
with both pointer recievers and non-pointer recievers. For the auth
policy, `getChallengeRequest` presumably did not have a pointer reciever
since it did not make use of the struct at all. We can simply move this
to a function.

Signed-off-by: Steve Kuznetsov <[email protected]>
auto-merge was automatically disabled August 5, 2024 14:11

Head branch was pushed to by a user without write access

@stevekuznetsov stevekuznetsov force-pushed the skuznets/method-to-function branch from 1b414de to f644995 Compare August 5, 2024 14:11
@stevekuznetsov
Copy link
Contributor Author

@tadelesh sorry, looks like auto-merge didn't go through. Updated the formatting, pushed again.

@tadelesh tadelesh merged commit ea67e9c into Azure:main Aug 6, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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