Skip to content

Commit

Permalink
Merge pull request #400 from GriffinPlus/master
Browse files Browse the repository at this point in the history 
Added explicit support for .NET 4.6.1 and .NET Core 2.0.
  • Loading branch information
@ericnewton76
ericnewton76 authored Jan 27, 2019
2 parents 795dc7b + 5f4c11d commit 0922e48
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/CommandLine/CommandLine.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<PropertyGroup>
<AssemblyName>CommandLine</AssemblyName>
<OutputType>Library</OutputType>
<TargetFramework>netstandard2.0</TargetFramework>
<TargetFrameworks>netstandard2.0; net461; netcoreapp2.0</TargetFrameworks>
<DefineConstants>$(DefineConstants);CSX_EITHER_INTERNAL;CSX_REM_EITHER_BEYOND_2;CSX_ENUM_INTERNAL;ERRH_INTERNAL;ERRH_DISABLE_INLINE_METHODS;CSX_MAYBE_INTERNAL;CSX_REM_EITHER_FUNC</DefineConstants>
<DefineConstants Condition="'$(BuildTarget)' != 'fsharp'">$(DefineConstants);SKIP_FSHARP</DefineConstants>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
Expand Down

6 comments on commit 0922e48

@moh-hassan
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericnewton76,
I find the PR contain this line:

             <TargetFrameworks>netstandard2.0; net461; netcoreapp2.0</TargetFrameworks>

CommandLineParser is used as a library (not an application), so the frameworks should exclude netcoreapp2.0

           <TargetFrameworks>net461; netstandard2.0</TargetFrameworks>

Have a look for this discussion

If you plan to add support for net40/net45 as given in my PR, supported fw can be:

              <TargetFrameworks>net40;net45;net461; netstandard2.0</TargetFrameworks>

Just a note, most Help System tests use SetAttributeOverride

The method SetAttributeOverride bypass the the method GetExecutingOrEntryAssembly which is part of the source of exceptions in unit test in net461 or above.

I suggest that all tests that test help system should be refactored and avoid using SetAttributeOverride and tested in all supported framework. So Test project should also be a multi-target project net461;netstandrad2;(net45,net40).

@ravenpride
Copy link
Contributor

@ravenpride ravenpride commented on 0922e48 Jan 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally you're right - but netcoreapp2.0 has nothing to do with applications or libraries. It only identifys the target framework as .NET Core 2.0. I've made another demo project targetting netcoreapp2.0 that pulls in the netstandard2.0 version of the CommandLineParser library. No additional interface assemblies are copied in this case. We should really drop the explicit support for netcoreapp2.0 as you suggested as it adds no additional value and increases the size of the Nuget package only.

Your suggestion concerning support of net40 will not work - at least not without changes. .NET 4.0 does not provide some of the reflection APIs used in the CommandLineParser library. But net45 seems to work fine, so it could be an idea to target net45 instead of net461 to increase the audience that can use the library.

@moh-hassan
Copy link
Collaborator

@moh-hassan moh-hassan commented on 0922e48 Jan 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I target net40 in my PR for backward compatibility of v 2.3.0. I did a minimal changes and it's working fine, and net40 is still needed for old projects.

I think that also net461 can exist together with net45 for the reason you show in your PR Added explicit support for .NET 4.6.1 .

For a project target net461:

  • Nuget will select net461 as the nearest lib in case the package contain net461.
  • If net461 isn't available it will select netstandard2 (not net45).

Have a look to Matching assembly versions and the target framework in a project, it's exact as our scenario.

@ravenpride
Copy link
Contributor

@ravenpride ravenpride commented on 0922e48 Jan 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, don't know what was wrong with me yesterday... Sorry for the confusion.

For a project target net461:

  • Nuget will select net461 as the nearest lib in case the package contain net461.
  • If net461 isn't available it will select netstandard2 (not net45).

Yep, you're right as .NET 4.6.1 complys with .NET Standard 2.0 and .NET 4.5 only complys with .NET Standard 1.1 (see here). Therefore a .NET 4.6.1 should really use the netstandard2.0 version and not the net45 version.

With your PR everybody should be happy - hopefully @ericnewton76 as well *looking over to him* :-)

@moh-hassan
Copy link
Collaborator

@moh-hassan moh-hassan commented on 0922e48 Jan 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ravenpride
Thank you very much.
Good point that you show net45 complys with .NET Standard 1.1.
So, CommandLineParser comply with netstandard1.1 up to netstandard2.0 +net461 👍

@ericnewton76
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll put out a release today, version bump to 2.5 I havent had time to keep up with the various gyrations of NetFx NetCore NetStandard and I see its still a bit of a mess.

Thankfully you guys have it covered. Thanks so much to @moh-hassan and @ravenpride for this.

Please sign in to comment.