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

Add SDK files to setup.py for EventGrid #4221

Merged
merged 7 commits into from
Aug 15, 2017
Merged

Conversation

kalyanaj
Copy link
Contributor


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

General Guidelines

  • The PR has modified HISTORY.rst with an appropriate description of the change (see Modifying change log).

Command Guidelines

  • Each command and parameter has a meaningful description.
  • Each new command has a test.

(see Authoring Command Modules)

@msftclas
Copy link

@kalyanaj,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@codecov-io
Copy link

codecov-io commented Aug 15, 2017

Codecov Report

Merging #4221 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4221   +/-   ##
=======================================
  Coverage   70.09%   70.09%           
=======================================
  Files         475      475           
  Lines       29603    29603           
  Branches     4539     4539           
=======================================
  Hits        20749    20749           
- Misses       7405     7409    +4     
+ Partials     1449     1445    -4
Impacted Files Coverage Δ
...dback/azure/cli/command_modules/feedback/custom.py 31.25% <0%> (ø) ⬆️
...li-cloud/azure/cli/command_modules/cloud/custom.py 13.79% <0%> (ø) ⬆️
src/azure-cli-core/azure/cli/core/util.py 70.66% <0%> (ø) ⬆️
...nent/azure/cli/command_modules/component/custom.py 16.26% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update efae17d...a4653bd. Read the comment docs.

'azure.cli.command_modules.eventgrid',
'azure.cli.command_modules.eventgrid.sdk',
'azure.cli.command_modules.eventgrid.sdk.models',
'azure.cli.command_modules.eventgrid.sdk.operations'
Copy link
Member

Choose a reason for hiding this comment

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

@derekbekoe out of curiosity, why didn't the CI catch this? Doesn't it verify each command package can be installed successfully?

Copy link
Member

Choose a reason for hiding this comment

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

Yes so it was installed successfully.
Since the imports don't happen until runtime, it didn't catch it.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm... if the module installation step ran "az eventgrid -h" instead of "az -h" would that have triggered the imports? If so, perhaps we should beef up our CI script to do that.

'azure.cli.command_modules.eventgrid',
'azure.cli.command_modules.eventgrid.sdk',
'azure.cli.command_modules.eventgrid.sdk.models',
'azure.cli.command_modules.eventgrid.sdk.operations'
Copy link
Member

Choose a reason for hiding this comment

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

Yes so it was installed successfully.
Since the imports don't happen until runtime, it didn't catch it.

@derekbekoe derekbekoe merged commit c3d8824 into Azure:master Aug 15, 2017
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.

6 participants