hvanbakel / CsprojToVs2017

Tooling for converting pre 2017 project to the new Visual Studio 2017 format.
MIT License
1.08k stars 120 forks source link

Project2015to2017 nuget library doesn't specify required nuget packages #238

Closed mungojam closed 5 years ago

mungojam commented 5 years ago

I thought I'd try upgrading my custom converter to the latest version but I found that I had to manually add a package reference to Microsoft.Extensions.Logging.Abstractions which is clearly a requirement of the library but isn't specified as such on nuget

It also requires CodeAnalysis and nuget.configuration libraries. Not sure why they haven't ended up in the dependencies automatically.

I should be able to get it up and running soon, looking forward to seeing how it does :)

andrew-boyarshin commented 5 years ago

Because to preserve Project2015To2017 library being a single NuGet package (not my choice) I had to include some MSBuild trickery. This is a bit fragile. I would've just created more NuGet packages and replaced primary one with a dummy for dependencies' sake. Visual Studio does it like this (lots of NuGet packages), I see no problem in that.

hvanbakel commented 5 years ago

We might do so in the near future now that we've given people a migration path where the tool indicates what the replacement package is. At the time that wasn't the case yet.

The global tool is now downloaded over 10 times more often than the original library. I would give it another month or so and then make the cut we've kept the old package alive long enough at that point.

On Sun, Mar 24, 2019, 18:35 Andrew Boyarshin notifications@github.com wrote:

Because to preserve Project2015To2017 library being a single NuGet package (not my choice) I had to include some MSBuild trickery. This is a bit fragile. I would've just created more NuGet packages and replaced primary one with a dummy for dependencies' sake. Visual Studio does it like this (lots of NuGet packages), I see no problem in that.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/hvanbakel/CsprojToVs2017/issues/238#issuecomment-476025645, or mute the thread https://github.com/notifications/unsubscribe-auth/AD8FPL-HocEFP8FqRSnra7RS9l_q37Acks5vaCflgaJpZM4cFrUa .

mungojam commented 5 years ago

I'll try adding the global tools as a nuget package. Perhaps the badge on the readme could be removed since that was what I used to see if the old packaged was still the recommended way.

andrew-boyarshin commented 5 years ago

You mean adding the global tool as a NuGet package? This can't be right. The correct way now is to reference the Project2015To2017 NuGet (as before, but care about implicit dependencies), although I would've probably just added this repo as git submodule and used ProjectReference directly. The way to go in the future (4.1?) would be creating separate NuGet packages for each project.

mungojam commented 5 years ago

Looks like I may need to give up on this version and go back to a much older one. There are various things that don't seem to work anymore when consuming as a package. For example I had a custom transform that was adding to project.AssemblyAttributeProperties, however they are no longer getting written out to the project file by the writer. I'm also clearing out the existing version tags but they are getting included anyway.

I expect I need to do it through the default property group, but unfortunately I don't have the time to work through it at this stage so I'll just go back to the older version instead.

hvanbakel commented 5 years ago

FYI, the build failed so none of the artifacts were created, looking at that now.

On Sun, Jun 2, 2019 at 1:46 PM Mark Adamson notifications@github.com wrote:

Closed #238 https://github.com/hvanbakel/CsprojToVs2017/issues/238 via

243 https://github.com/hvanbakel/CsprojToVs2017/pull/243.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/hvanbakel/CsprojToVs2017/issues/238?email_source=notifications&email_token=AA7QKPFR3VHEZHFDVW26ECDPYQURVA5CNFSM4HAWWUNKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGORYHWXWI#event-2383375321, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7QKPEOE64HNF6GTAB4KNDPYQURVANCNFSM4HAWWUNA .

hvanbakel commented 5 years ago

And it went through now, it's published as a beta version

On Sun, Jun 2, 2019 at 3:25 PM Hans van Bakel jwvanbakel@gmail.com wrote:

FYI, the build failed so none of the artifacts were created, looking at that now.

On Sun, Jun 2, 2019 at 1:46 PM Mark Adamson notifications@github.com wrote:

Closed #238 https://github.com/hvanbakel/CsprojToVs2017/issues/238 via

243 https://github.com/hvanbakel/CsprojToVs2017/pull/243.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/hvanbakel/CsprojToVs2017/issues/238?email_source=notifications&email_token=AA7QKPFR3VHEZHFDVW26ECDPYQURVA5CNFSM4HAWWUNKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGORYHWXWI#event-2383375321, or mute the thread https://github.com/notifications/unsubscribe-auth/AA7QKPEOE64HNF6GTAB4KNDPYQURVANCNFSM4HAWWUNA .