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

Support of target runtime v4.7.1 #8

Merged
merged 1 commit into from
Nov 15, 2017
Merged

Support of target runtime v4.7.1 #8

merged 1 commit into from
Nov 15, 2017

Conversation

baSSiLL
Copy link
Contributor

@baSSiLL baSSiLL commented Nov 14, 2017

I've added corresponding element to the targets file for VS 2017.

@Igorbek
Copy link
Owner

Igorbek commented Nov 15, 2017

Thank you the pull request!
We can include this change, but generally I prefer to update Code Contracts library.
Is that or similar change landed in Code Contracts repo?
If not, you can make a pull request there and then we can include it here.
I'd like this project to not take responsibilities of developing Code Contracts itself.

@baSSiLL
Copy link
Contributor Author

baSSiLL commented Nov 15, 2017

This is a minor change considering you've already included support for VS2017 and v4.7 target framework. That is, you are already beyond the latest stable release of Code Contracts in these terms. And this is good as allows to use Code Contracts in new projects being developed with VS 2017 and the latest .NET versions.
If you are OK to integrate changes that have not yet been released as stable yet, then it would be better to include contract definitions for BCL 4.6 which are included in the latest RC of Code Contracts. So they are used when compiling for target frameworks v4.6+ (currently v4.5 definitions are used in your targets).
Unfortunately, Code Contracts don't have support even for v4.7 in their targets (yet having targets for VS 2017). I think the reasoning behind that is there are no contract definitions for BCL v4.7 (the latest in repo and RC are for 4.6), so the support won't be "complete".

@Igorbek Igorbek merged commit 5d9b69b into Igorbek:master Nov 15, 2017
@Igorbek
Copy link
Owner

Igorbek commented Nov 15, 2017

Yes, that makes sense. Thank you for the contribution, I'll push the package shortly.

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.

2 participants