Skip to content

Conversation

@codemzs
Copy link
Member

@codemzs codemzs commented Jan 16, 2019

Adds a new CI leg just for code coverage. The reason for adding a new leg is not let code coverage process affect PR merge.

@codemzs codemzs requested a review from safern January 16, 2019 22:48

- ${{ if eq(variables['codeCoverageBuild'], 'true') }}:
- phase: ${{ parameters.name }}
variables:
Copy link
Member

Choose a reason for hiding this comment

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

I think variables can be common across the two blocks

Copy link
Contributor

@sharwell sharwell left a comment

Choose a reason for hiding this comment

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

We should confirm that the performance overhead of running code coverage alongside one of the current debug builds is substantial before breaking it out into its own leg. Performance testing is currently blocked while we wait for the next release of coverlet.

- phase: ${{ parameters.name }}
- ${{ if ne(variables['codeCoverageBuild'], 'true') }}:
- phase: ${{ parameters.name }}
variables:
Copy link
Member

Choose a reason for hiding this comment

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

You would need to indent everything under these if blocks in this file if you go with the current structure of the phase blocks.

That is why the build is failing:

phase-template.yml (Line: 11, Col: 5, Idx: 199) - (Line: 11, Col: 5, Idx: 199): While parsing a block collection, did not find expected '-' indicator. 

phases:
- ${{ if ne(variables['codeCoverageBuild'], 'true') }}:
${{ if ne(variables['codeCoverageBuild'], 'true') }}:
Copy link
Member

Choose a reason for hiding this comment

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

the - are needed at the beginning of this ifs because phases expect objects to be declared. In yaml you specify an object with - the error you're getting I described it in another comment.

@codemzs
Copy link
Member Author

codemzs commented Jan 17, 2019

@sharwell If the new nuget does not affect the performance much then this PR can be closed. The current nuget is very slow and causes build timeouts. We are expecting the new nuget to be available tomorrow.

CC: @shauheen @eerhardt

@safern safern closed this Jan 17, 2019
@safern safern reopened this Jan 17, 2019
@safern safern closed this Jan 17, 2019
@safern safern reopened this Jan 17, 2019
@safern safern closed this Jan 17, 2019
@shauheen shauheen deleted the ccbuilddef branch January 23, 2019 22:48
@ghost ghost locked as resolved and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

4 participants