Skip to content

Conversation

@buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Mar 6, 2020

Contributes to #2339

CC @dotnet/ncl

@buyaa-n buyaa-n added this to the 5.0 milestone Mar 6, 2020
@Tratcher
Copy link
Member

Tratcher commented Mar 6, 2020

What's going to happen when then the changes in the shared files sync to aspnetcore which does not use nullable? Please have someone try it before merging this.

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Mar 6, 2020

What's going to happen when then the changes in the shared files sync to aspnetcore which does not use nullable?

By default we are enabling nullability on project level and if there is any shared file which failing other projects because of a ? annotation, we add #nullable enable directive to the file, then the other references would be fine

Please have someone try it before merging this.

Please let me know who could help with that and/or give us the list of referenced files, then I can check them and add #nullable enable if needed

@stephentoub stephentoub requested a review from a team March 6, 2020 16:42
@eiriktsarpalis
Copy link
Member

/azp list

@eiriktsarpalis
Copy link
Member

/azp run runtime-libraries outerloop

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

Copy link
Member

@eiriktsarpalis eiriktsarpalis left a comment

Choose a reason for hiding this comment

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

LGTM in general. Other than addressing feedback, due to the large number of asserts being added, could you please ensure that outerloop tests are working as expected?

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Mar 14, 2020

due to the large number of asserts being added, could you please ensure that outerloop tests are working as expected?

Sure i can run outerloop, JFYI i was able to run only Functional and Unit tests, there were no outerloop tests for Unit tests, all outerloop tests passed for Functional tests.

Didn't run and don't know how to run Enterprise and Stress tests

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Mar 16, 2020

What's going to happen when then the changes in the shared files sync to aspnetcore which does not use nullable? Please have someone try it before merging this.

@Tratcher this PR pretty close to be merged, who from aspnetcore team could try this out before merge?

@Tratcher
Copy link
Member

Sorry, I've been OOF. I can give this a try today.

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Mar 16, 2020

Sorry, I've been OOF. I can give this a try today.

Thanks, please let me know if something fail, otherwise this PR is good to go

@buyaa-n buyaa-n merged commit 4b03513 into dotnet:master Mar 17, 2020
@buyaa-n
Copy link
Contributor Author

buyaa-n commented Mar 17, 2020

@dotnet/ncl if you have a PR updating this API please pull and build first to avoid build failure in repo, even if you are not seeing conflict or build error on your branch

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants