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

Adding CG support, moving analyze_deps to analyze job #4345

Merged
merged 18 commits into from
Feb 14, 2019

Conversation

KarishmaGhiya
Copy link
Member

@Azure/azure-sdk-eng

# This is the 1st commit message:

Resolved merge conflict by incorporating both suggestions

# This is the commit message Azure#2:

indentation fixed

# This is the commit message Azure#3:

indentation fixed

# This is the commit message Azure#4:

indentation fixed
@adxsdk6
Copy link

adxsdk6 commented Feb 12, 2019

Can one of the admins verify this patch?

1 similar comment
@adxsdk6
Copy link

adxsdk6 commented Feb 12, 2019

Can one of the admins verify this patch?

@codecov-io
Copy link

codecov-io commented Feb 12, 2019

Codecov Report

Merging #4345 into master will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4345      +/-   ##
==========================================
+ Coverage   53.45%   53.53%   +0.07%     
==========================================
  Files       10449    10452       +3     
  Lines      218338   218687     +349     
==========================================
+ Hits       116717   117066     +349     
  Misses     101621   101621
Impacted Files Coverage Δ
...osearch/azure/cognitiveservices/search/__init__.py 100% <0%> (ø)
azure-mgmt-documentdb/azure/mgmt/__init__.py 100% <0%> (ø)
...computervision/azure/cognitiveservices/__init__.py 100% <0%> (ø)
...rvicebus/azure/servicebus/control_client/models.py 94.8% <0%> (+0.06%) ⬆️
...ure/servicebus/control_client/servicebusservice.py 93.07% <0%> (+0.11%) ⬆️
...zure-sdk-tools/devtools_testutils/mgmt_testcase.py 93.93% <0%> (+0.18%) ⬆️
...e-keyvault/azure/keyvault/http_bearer_challenge.py 89.28% <0%> (+0.19%) ⬆️
...azure/servicemanagement/servicemanagementclient.py 84.53% <0%> (+0.24%) ⬆️
azure-keyvault/azure/keyvault/key_vault_client.py 75% <0%> (+0.26%) ⬆️
...rce/azure/mgmt/resource/features/feature_client.py 93.33% <0%> (+0.31%) ⬆️
... and 26 more

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 6b307b4...2a77fb2. Read the comment docs.

Copy link
Member

@scbedd scbedd left a comment

Choose a reason for hiding this comment

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

Glad to see the CG stuff works with python. Make these adjustments!

.azure-pipelines/client.yml Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
@KarishmaGhiya KarishmaGhiya changed the title resolved conflict, updated job analyze with dependency steps Adding CG support, moving analyze_deps to analyze job Feb 12, 2019
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
.azure-pipelines/client.yml Outdated Show resolved Hide resolved
os.name: 'Linux'
os.vmImage: 'ubuntu-16.04'
python.version: '2.7'
OSName: 'Linux'
Copy link
Member

Choose a reason for hiding this comment

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

@scbedd it doesn't even look like we use the OSName anywhere so we should probably just delete it. @mikeharder I see you do use OsName as part of a task description in the JS pipeline. Given that the strategy name is used in the job name I personally don't feel it is worth plumbing this extra information through. What do others think?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. everywhere in the template we use os.name.

Copy link
Member

Choose a reason for hiding this comment

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

I must be missing the usages then as I don't see "os.name" or "OSName" being used anywhere in this pipeline.

Copy link
Member

Choose a reason for hiding this comment

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

You are correct. It's been updated.

@scbedd scbedd merged commit e5b7af7 into Azure:master Feb 14, 2019
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.

7 participants