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

netcoreapp2.0 packages are compatibile with netcoreapp2.1 #3336

Merged
merged 3 commits into from Aug 16, 2018

Conversation

Projects
None yet
3 participants
@enricosada
Collaborator

enricosada commented Aug 15, 2018

the netcoreapp2.1 framework support netcoreapp2.0 packages

@matthid

nice catch

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 15, 2018

Member

I'm curious: Did you notice an impact of this? It looks like something incredible hard to debug, but I can't think of what issues would arise from this...

Member

matthid commented Aug 15, 2018

I'm curious: Did you notice an impact of this? It looks like something incredible hard to debug, but I can't think of what issues would arise from this...

@enricosada

This comment has been minimized.

Show comment
Hide comment
@enricosada

enricosada Aug 15, 2018

Collaborator

@matthid added cleaned up + test

i found it working on my next PR, i just splitted it so is easier to review.
The issue was a target framework condition was not populated correctly, but yes, is really small impact (really few netcoreapp2.0 packages out there)

Collaborator

enricosada commented Aug 15, 2018

@matthid added cleaned up + test

i found it working on my next PR, i just splitted it so is easier to review.
The issue was a target framework condition was not populated correctly, but yes, is really small impact (really few netcoreapp2.0 packages out there)

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 15, 2018

Member

Looks good to me. Thanks for the tests!

Member

matthid commented Aug 15, 2018

Looks good to me. Thanks for the tests!

@enricosada

This comment has been minimized.

Show comment
Hide comment
@enricosada

enricosada Aug 15, 2018

Collaborator

I'll merge so when build is green.
Yes, was really fun to debug 😄

Collaborator

enricosada commented Aug 15, 2018

I'll merge so when build is green.
Yes, was really fun to debug 😄

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 15, 2018

Member

I'll merge so when build is green.

Good luck with that ;)

Member

matthid commented Aug 15, 2018

I'll merge so when build is green.

Good luck with that ;)

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Aug 16, 2018

Member

it's rather important fix. I'll release. Thanks!

Member

forki commented Aug 16, 2018

it's rather important fix. I'll release. Thanks!

@matthid

This comment has been minimized.

Show comment
Hide comment
@matthid

matthid Aug 16, 2018

Member

Honestly I'm a bit worried that our reviews didn't catch this, but good that it is solved now.

Member

matthid commented Aug 16, 2018

Honestly I'm a bit worried that our reviews didn't catch this, but good that it is solved now.

@forki forki merged commit 1ff1776 into fsprojects:master Aug 16, 2018

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment