Skip to content

Remove automation parameters from client #2741

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.

Sign up for GitHub

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

Jump to bottom
Merged
lmazuel merged 1 commit into from
Mar 23, 2018
Merged

Remove automation parameters from client #2741

lmazuel merged 1 commit into from
Mar 23, 2018

Conversation

@vladbarosan
Copy link

@vladbarosan vladbarosan commented Mar 23, 2018 •
edited by azuresdkci
Loading

Fixes Azure/azure-sdk-for-go#981

  • moving global parameters clientRequestId , automationAccountName and resourceGroupName

@AutorestCI
Copy link

AutorestCI commented Mar 23, 2018 •
edited
Loading

Automation for azure-libraries-for-java

Nothing to generate for azure-libraries-for-java

@AutorestCI AutorestCI mentioned this pull request Mar 23, 2018
Merged
@AutorestCI
Copy link

AutorestCI commented Mar 23, 2018 •
edited
Loading

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#1959

@vladbarosan vladbarosan requested a review from najams March 23, 2018 01:22
@vladbarosan
Copy link
Author

vladbarosan commented Mar 23, 2018

@najams can you please have a look at those parameters why they are needed in the client ? I moved them to be in the method. Or let me know if it makes sense to be in the client and not in the methods. But right now they are in both places and it doesnt make sense.

@vladbarosan vladbarosan requested a review from mcardosos March 23, 2018 01:23
@AutorestCI AutorestCI mentioned this pull request Mar 23, 2018
Merged
@AutorestCI
Copy link

AutorestCI commented Mar 23, 2018 •
edited
Loading

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#1411

@AutorestCI AutorestCI mentioned this pull request Mar 23, 2018
Merged
@AutorestCI
Copy link

AutorestCI commented Mar 23, 2018 •
edited
Loading

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#2483

lmazuel
lmazuel approved these changes Mar 23, 2018
View reviewed changes
Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

Definitely the right thing to do :)

@lmazuel
Copy link
Member

lmazuel commented Mar 23, 2018

Looking at the Python code, this was indeed broken in Python as well, and I'm 110% sure this is the right fix, so I merge it asap to fix everybody asap. @najams if you have any concerns on this choice, please reach out to us, always open to discussion :)

@lmazuel lmazuel merged commit f7541bf into Azure:master Mar 23, 2018
@vladbarosan vladbarosan deleted the fixAutomationClient branch November 21, 2018 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lmazuel lmazuel lmazuel approved these changes

@najams najams Awaiting requested review from najams

@mcardosos mcardosos Awaiting requested review from mcardosos

Assignees

@lmazuel lmazuel

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vladbarosan @AutorestCI @lmazuel @alvadb