Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

Conversation

@natemcmaster
Copy link
Contributor

This puts all dependency versions into one file that his shared will all projects. This makes it easier to ensure versions are consistent.

@SteveSandersonMS I made this a separate PR from #721 as the change is not essential to the VS 2017 upgrade. We are moving towards this model in our other aspnet repos.

@dnfclas
Copy link

dnfclas commented Feb 28, 2017

@natemcmaster,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Mar 1, 2017

Thanks @natemcmaster! This is a big improvement and is now merged.

I did edit the commit a little when merging. I reverted the changes to the templates projects .csproj files (e.g, Angular2Spa, ReactSpa, etc.). That's because those aren't just regular .NET projects - they are the input to the system that builds Yeoman and dotnet new templates. As given, the tokenised values (e.g., $(AspNetCoreVersion) would have ended up in the template packages, which then would have generated projects that can't compile (because the dependencies.props file wouldn't be there). Also there's an argument that the dependency versions we use in the templates don't need to be the same as we use elsewhere, and we need to make separate decisions about when to change those dependency versions. So overall I think it's preferable to continue specifying those versions explicitly and independently. Hope that's OK!

@SteveSandersonMS SteveSandersonMS deleted the namc/deps branch March 1, 2017 09:34
@natemcmaster
Copy link
Contributor Author

natemcmaster commented Mar 1, 2017

Ah, thanks for clarifying. I wasn't 100% sure that the template projects were for. Thanks for cleaning it up 👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants