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

Method renames for dotnet automation API GA #6731

Merged
merged 2 commits into from
Apr 8, 2021

Conversation

t0yv0
Copy link
Member

@t0yv0 t0yv0 commented Apr 8, 2021

Takes care of the part of #6459 as relates to method renames. In addition renames RemoveConfigAsync and RemoveAllConfigAsync.

@t0yv0 t0yv0 requested a review from komalali April 8, 2021 20:27
@t0yv0 t0yv0 added the impact/breaking Fixing this issue will require a breaking change label Apr 8, 2021
@t0yv0
Copy link
Member Author

t0yv0 commented Apr 8, 2021

CC @orionstudt

@t0yv0 t0yv0 merged commit e460ab7 into master Apr 8, 2021
@t0yv0 t0yv0 deleted the t0yv0/dotnet-automation-api-ga-renames branch April 8, 2021 22:01
@orionstudt
Copy link
Contributor

I will say that I'm not a fan of these names... I had thought that in the original Automation API PR we had agreed to go with names that unify related methods alphabetically for intellisense and usage purposes here

@t0yv0
Copy link
Member Author

t0yv0 commented Apr 8, 2021

Thanks for that pointer @orionstudt and the feedback. I think the current priority is cross-SDK consistency of the operation names so the decision was made to got his way. I will take a note to bring this up again with the team so we may reconsider. Especially since consistency can be solved the other way, that is editing the other SDKs.

@komalali
Copy link
Member

komalali commented Apr 8, 2021

@orionstudt Ah, I definitely missed that entire conversation.

The issues for the other languages were never opened, and I wasn't aware that we had decided that we would align the other languages with C# rather than the other way around.

At this point, it wouldn't be entirely painless to take this breaking change in the 3 other SDKs, not to mention the related changes in the code that consumes these APIs. If we were to do it, we need to somehow fit that into the next 12 days which I'm not sure we have the capacity for.

@orionstudt
Copy link
Contributor

orionstudt commented Apr 8, 2021

@komalali it is what it is! I don't think it is that big of an issue, but I do think there is value in related methods being near each other alphabetically for intellisense. I think I tried to clarify the miscommunication here but I failed to be specific enough or open the other issues so its on me 😅

abhinav pushed a commit to pulumi/pulumi-dotnet that referenced this pull request Jan 11, 2023
* Method renames

* Add CHANGELOG_PENDING entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/breaking Fixing this issue will require a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants