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

Fix support for .NET 4.0 and .NET 4.5 #257

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

Athari
Copy link
Contributor

@Athari Athari commented Mar 20, 2018

Covers #222 and #227.

I would have fixed .nuspec more throroughly, but I can't even get .NET Standard 1.5 library to build, so this is the only change I can do reliably without breaking anything.

See #227 (comment) for more details on the issues with .nuspec file and the following comment with a way to fix it.

@ericnewton76
Copy link
Member

This looks valid.

I need to figure out how to get Appveyor to do a little more related to these types of changes for smoke testing. I'm thinking example programs that are targeting Framework v4.0 and Framework v4.5,v4.6,v4.7 and so on and have appveyor/ci build run nuget install or update -r on these example programs to see if theres some install dependency problems.

Did you already create two test apps that are targeting Fx v4.0 and v4.5? Maybe send a pull request on the examples repo to add those, under targeting_tests subdirectory.

I mentioned using the examples repo because my first instinct is let them version independently from the source repo (commandline) to help simulate real world scenarios

@Athari
Copy link
Contributor Author

Athari commented Mar 28, 2018

@ericnewton76 I haven't tested anything, I just copied the required lines from another .nuspec.

I can't get the current build scripts to work properly and don't have access to CI artifacts (if AppVeyor publishes .nupkg), so I can't actually test anything.

If you give me carte blanche on the build and project system, I can perform the following changes:

  1. replace Paket dependency manager with NuGet;
  2. remove dependencies on single files by moving them to repo itself;
  3. convert .csproj to new SDK format with multi-targeting and automatic NuGet packaging;
  4. remove strong assembly naming and possibly add it as a separate package;
  5. update AppVeyor scripts accordingly.

While smoke testing is a good thing, these changes alone would:

  1. produce reliable NuGet packages as .nuspec would be generated from project files instead of being written by hand;
  2. make the build system more accessible to other developers as no third-party tools would be required;
  3. make the package follow the usual recommendations (including not signing);
  4. bump minimum VS version requirement to VS2017, where new .csproj format is supported.

I consider the code of the library itself to be in the "doomed" state, as it's written in a hacky F#-in-C# style which nobody except the original author uses and depends on a low-quality base library, and replacing all "maybe" nonsense with normal code, fixing all IEnumerable usages, getting rid of all pointless inverse-of-control and pattern matching imitation throughout API etc. would be a major effort. However, some things can be improved without major changes and I think it's worth the effort as the library is still very popular.

@gbritton1
Copy link
Contributor

+1 except for vs 2017 dependency. My team will be on 2015 for a while.

Currently though I can't build the solution which is a real drag. Specifically the test project.

@Athari
Copy link
Contributor Author

Athari commented Mar 28, 2018

@gbritton1 VS2017 is a library development dependency. NuGet and app dependencies remain the same. Now that fully featured VS Community is free, I see no point in not switching to new VS version in relatively small open-source projects where it can be done without any hassle.

The rest of changes can't be done without switching to VS2017 anyway. Multi-tartgeting and NuGet package generation on build are features of new SDK based project system of VS2017.

@gbritton1
Copy link
Contributor

@Athari well then, let's get on with it!

@Lakritzator
Copy link

I was really excited about commandlineparser 2.3.0 arriving yesterday!
... until I found out that this change is not yet in there.

What is the current plan for this?

I have the same issue like many others, trying to release a .NET framework 4.6.1 application, but using this package forces many additional unneeded system dependencies which bloat the application.

@ericnewton76
Copy link
Member

ericnewton76 commented Aug 15, 2018

We're close to releasing a NetStandard20 based. A Microsoft fellow (Viktor Hofer) did some work to bring the library to that level which minimizes the required dependencies from the NetStandard side.

On the NetFramework side, should we push it back to targeting v4.0? Would that support this need better?

UPDATE: I just realized this PR does add that. I'm (re-)looking into it now.

@Lakritzator
Copy link

@ericnewton76 This is about the nuspec only targeting netstandard 1.5 and not net40 / net45.
This result is that the nuget package is used at netstandard 1.5, and when you use 4.6.1 this causes the netstandard dependencies to be added.

As the project seems to be for net40, this doesn't make sense.

This PR should fix the targeting issue, which is a simple change and solves an issue which will help a lot of people.

@ericnewton76
Copy link
Member

Ok, I gotcha. I should've included this in v2.3. I will bump to v2.4 and probably deprecate v2.3.0

@ericnewton76 ericnewton76 changed the base branch from master to develop August 16, 2018 14:39
@ericnewton76 ericnewton76 merged commit d6be4f9 into commandlineparser:develop Aug 16, 2018
@jsreynolds
Copy link

Any update on this by chance? I had to change the .nuspec on the 2.3 to get it to play nicely with .Net 4.6.1.

Thanks,

--J

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.

5 participants