Skip to content

Conversation

@julienstroheker
Copy link
Contributor


This checklist is used to make sure that common guidelines for a pull request are followed.

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@julienstroheker
Copy link
Contributor Author

CC @jim-minter @mjudeikis @amanohar

@tjprescott tjprescott requested a review from mboersma April 12, 2019 15:47
Copy link
Member

@mboersma mboersma left a comment

Choose a reason for hiding this comment

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

lgtm, I just had a question about LongRunningOperation.

resource_group_name=resource_group_name, resource_name=name, parameters=osamc)
result = sdk_no_wait(no_wait, client.create_or_update,
resource_group_name=resource_group_name, resource_name=name, parameters=osamc)
result = LongRunningOperation(cmd.cli_ctx)(result)
Copy link
Member

Choose a reason for hiding this comment

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

We don't use result after this point in this method. Should it be returned? Why are we making a LongRunningOperation out of it if it isn't used?

===============
2.3.22
++++++
* Removing `--fqdn` flag on az openshift commands
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change and cannot be done with a simple patch version. This needs to call out the breaking change and be bumped to version 2.4.0

@tjprescott tjprescott mentioned this pull request Apr 16, 2019
2 tasks
# Conflicts:
#	src/command_modules/azure-cli-acs/HISTORY.rst
#	src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/_help.py
#	src/command_modules/azure-cli-acs/azure/cli/command_modules/acs/custom.py
@julienstroheker
Copy link
Contributor Author

@tjprescott can you give another look please ? Thanks

@tjprescott tjprescott merged commit 6f12b8f into Azure:dev Apr 26, 2019
@julienstroheker julienstroheker deleted the osa-remove-fqdn-param branch April 27, 2019 01:35
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