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

Branch out DotNet 5 #1484

Merged
merged 12 commits into from
Sep 1, 2020
Merged

Branch out DotNet 5 #1484

merged 12 commits into from
Sep 1, 2020

Conversation

ooooolivia
Copy link
Contributor

No description provided.

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

Overall LGTM, but I think that it would be great to have at least two CI legs that test 6.0 bits (master channel)

I've reported dotnet/runtime#41521 to unblock this PR.

@ooooolivia thank you for working on this!

azure-pipelines.yml Show resolved Hide resolved
'5.0':{
'tfm': 'netcoreapp5.0',
'branch': 'release/5.0'
},
'release/3.1.3xx':{
Copy link
Member

Choose a reason for hiding this comment

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

not related to the goal of this PR, but should we add release/3.1.4xx to the list as well? (we are currently using release/3.1.3xx)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing it out! Yes I think we should replace release/3.1.3xx with release/3.1.4xx now as there's no active development on the former.. will include the change in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just realized feeds for release/3.1.4xx are internal feeds different for each build so it cannot simply be hard-coded in NuGet.config. We've discussed it before and the possible solution is to query darc (seems to require more credential setup) to get a new feed each time in our pipeline. @billwert @DrewScoggins Do we have any next step regarding this?

Copy link
Member

Choose a reason for hiding this comment

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

@ooooolivia FWIW if release/3.1.4xx is problematic we can just keep using the release/3.1.3xx channel. To me, the biggest value of this PR is that it allows me to benchmark 5.0 bits produced from the RC branch, not the 6.0 stuff from master branch.

Copy link
Member

Choose a reason for hiding this comment

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

agreed. let's do as much as we can and sort the 3.1.4 thing out later.

scripts/dotnet.py Show resolved Hide resolved
@adamsitnik
Copy link
Member

@ooooolivia the feedback I got in dotnet/runtime#41521 tells me that Utf8String won't be included in 5.0 RTM. I think that for now to unblock this important PR you can conditionally exclude https://github.com/dotnet/performance/blob/master/src/benchmarks/micro/libraries/System.Utf8String/Perf.Utf8String.cs for netcoreapp5.0 and later we can find a proper solution

@adamsitnik
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@billwert billwert left a comment

Choose a reason for hiding this comment

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

Looks good, after you remove 3.1.4xx and go back to 3.1.3xx.

@ooooolivia ooooolivia merged commit cc6eb20 into dotnet:master Sep 1, 2020
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.

None yet

3 participants