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

[ConnectedK8s] Added update command (#3) #2091

Merged
merged 4 commits into from
Aug 10, 2020
Merged

Conversation

Anumita
Copy link
Contributor

@Anumita Anumita commented Jul 29, 2020


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

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

src/connectedk8s/azext_connectedk8s/custom.py Show resolved Hide resolved
src/connectedk8s/azext_connectedk8s/custom.py Show resolved Hide resolved
src/connectedk8s/azext_connectedk8s/custom.py Outdated Show resolved Hide resolved
src/connectedk8s/azext_connectedk8s/custom.py Outdated Show resolved Hide resolved
src/connectedk8s/azext_connectedk8s/custom.py Show resolved Hide resolved
src/connectedk8s/azext_connectedk8s/_utils.py Outdated Show resolved Hide resolved
@yonzhan yonzhan requested review from qwordy and arrownj July 30, 2020 22:31
@yonzhan
Copy link
Collaborator

yonzhan commented Jul 30, 2020

ConnectedK8s

@qwordy
Copy link
Member

qwordy commented Jul 31, 2020

@arrownj

@yungezz
Copy link
Member

yungezz commented Aug 5, 2020

hi @fengzhou-msft could you pls review this PR? thanks

Copy link

@krdhruva krdhruva left a comment

Choose a reason for hiding this comment

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

🕐

Comment on lines 21 to 23
c.argument('https_proxy', options_list=['--proxy-https'], help='Https proxy URL to be used.')
c.argument('http_proxy', options_list=['--proxy-http'], help='Http proxy URL to be used.')
c.argument('no_proxy', options_list=['--proxy-skip-destinations'], help='List of URLs/CIDRs for which proxy should not to be used.')
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. Please evaulate the impact to customers. You may need to keep the old option and add deprecation message before finally removing it if necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the last version was not published, so there are no customers that would be using it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1878, this was the last PR in which the change was added. The version had not been upgraded in setup.py, so i guess it would not have been published right?

@Anumita
Copy link
Contributor Author

Anumita commented Aug 7, 2020

@fengzhou-msft , @arrownj , Everything is set from our side. Could you review as well? If possible could we get this merged today? We wanted to release this feature next week for customers.

@fengzhou-msft fengzhou-msft merged commit e7b8be8 into Azure:master Aug 10, 2020
@fengzhou-msft
Copy link
Member

@Anumita version 0.2.3 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants