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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Net Standard support #96

Closed
wants to merge 10 commits into from
Closed

Conversation

zvirja
Copy link
Member

@zvirja zvirja commented Aug 28, 2017

Added .NET Standard 2.0 support. Closes #94.

Currently we are migrating AutoFixture to support .NET Core and it appeared that AutoFixture.Idioms has dependency on Albedo. Therefore, I decided to enable .NET Core support for this lib (to later consume that in AutoFixture).

Changes

The significant changes I did are described below:

  • Converted both projects to .NET Core SDK format.
  • Albedo now targets frameworks: .NET Framework 3.5 and .NET Standard 2.0. I tried to use .NET Standard 1.5 as a target, however the type.ReflectedType API used all over the project isn't present there. Given that I don't know whether it's vital or not, I decided to keep that API usage as is and target the .NET Standard 2.0 where this API is present.
  • Albedo.UnitTests now target frameworks .NET Framework 4.5.2 and .NET Core App 2.0. That is needed to run xUnit on both .NET Framework and .NET Core platforms.
  • Albedo.UnitTests now references the latest beta of xUnit 2.3 to be able to run tests on both platforms.
  • FxCop (code analysis) now is disabled for .NET Standard as it doesn't work. We still run Code Analysis for the .NET 3.5 target, so everything should be fine.
  • Removed most of the AssemblyInfo.cs file attributes and use automatic attributes generation. The source properties are being specified in the Albedo.csproj file.
  • Removed the nuspec file to use dotnet pack instead and generate package using the csproj file. See Albedo.csproj where some of the NuSpec properties are being specified now.
  • Removed the Packages directory from the repository. See explanation in this commit description.

Dev environment

To be able to open the solution and build it you need VS 2017 Update 3 and .NET Core SDK 2.0 installed. If you use AppVeyor, everything is installed there.

How to build

I found that currently project contains some of the build infrastructure and I decided to not touch it - so @ploeh will fix that manually as he knows it better. Currently, in order to build projects you need the following steps:

  • set the Src folder as a current dir in console;
  • dotnet build --configuration Verify- to run Code Analysis and ensure to warnings are present;
  • dotnet test Albedo.UnitTests - run xUnit tests in all the frameworks (.NET 3.5 and .NET Core 2.0);
  • dotnet pack --configuration Release /property:Version=<packageVersion> /property:FileVersion=<assemblyFileVersion> /property:InformationalVersion=<assemblyInformationalVersion> /property:AssemblyOriginatorKeyFile="<pathToSignKeyFile>" - to create a NuGet package based on the project file. You need to replace the placeholders:
    • <packageVersion> - set NuGet package version (e.g. 1.0.2);
    • <assemblyFileVersion> - set AssemblyFileVersion attribute value (e.g. 1.0.2.0);
    • <assemblyInformationalVersion> - set AssemblyInformationalVersion attribute value (e.g. 1.0.0.0);
    • <pathToSignKeyFile> - full path to the SNK file with key to sign the assembly.

Further steps

Currently I've done all that I wanted from my side an am looking forward to @ploeh's review. Also I haven't changed the build infrastructure (all those PS files and assembly patching that we no longer need) and would like to ask you to fix that by yourself (via separate commit after this PR is merged).


@ploeh Looking forward to your review and thank you for your time! 馃槒 If you feel that somebody else should be invited to review the PR - please invite 馃槈

P.S. I know that this PR is quite big, however I don't see a way to split it into the smaller chunks - everything should be changed at once..

@zvirja
Copy link
Member Author

zvirja commented Aug 28, 2017

@moodmosaic @adamchester JFYI 馃槈

@zvirja
Copy link
Member Author

zvirja commented Aug 31, 2017

@ploeh Did you have any chance to review the PR? Just to ensure that you haven't missed it..

@ploeh
Copy link
Collaborator

ploeh commented Sep 1, 2017

Thank you for the pull request. I'm sorry to say, however, that I don't intent to consider it for the time being.

Currently, I have no customers paying me to do .NET Core work, so I'm using my energy on other technologies. I'm not at all up to speed on the that stack, and I'm still on Visual Studio 2015, because of the continuing problems with Visual Studio 2017 and F#.

The reason we originally created Albedo was to support some abstractions in AutoFixture.Idioms, but since these abstractions were entirely decoupled from AutoFixture, it seemed more appropriate to keep it separate.

In practice, however, I don't know of that it's used by other libraries than AutoFixture...

In order to move forward, I suggest that you pull it into the AutoFixture organisation. I'll be fine with handing over the signature file and the NuGet credentials.

@zvirja
Copy link
Member Author

zvirja commented Sep 11, 2017

@ploeh It has been decided to not move this project under the AutoFixture account. Therefore, if you don't want to take care of this project more, I'd like to create a fork, add .NET Core support and publish it to NuGet as a package with different identity. The MIT license allows to do that without issues.

The first idea was to name it smth like "Albedo.XXX", where XXX will reflect that this is a fork. That would allow people to find it using the Albedo name. However, I'm unable to find a good "XXX" substitution 馃槚

Another idea is to perform a hard fork and create a completely different library and call it e.g. Reflectance. Of course, the code base will be the same - only namespace will be changed.

I still in doubts with approach is better 馃槙 Could you share your thoughts on this?

@ploeh
Copy link
Collaborator

ploeh commented Sep 12, 2017

You could also just create an Albedo organisation, and call it Albedo still... As I wrote previously, I'd be perfectly happy to hand off the project, including NuGet credentials and strong key, so that the identity of Albedo can move on without change.

And while I'm not a lawyer, I agree that the MIT licence should be enough to enable anyone to fork to their hearts content 馃槃

@zvirja zvirja deleted the add-net-core-support branch September 12, 2017 15:34
@zvirja
Copy link
Member Author

zvirja commented Sep 13, 2017

Hi Mark,

I've followed your idea and created an organization for the project. See the organization project here: https://github.com/AlbedoOrg

Could you please:

  1. Transfer this repository to the https://github.com/AlbedoOrg account? I want to preserve all the PRs and issues for the better history.
  2. Push the sign key to the Src\Albedo.snk. In AutoFixture we don't keep it secret and I want to follow the same idea.
  3. Add me as a package owner on NuGet.

That will pass ownership to that org :)

I'm going to apply the following changes:

  1. Perform housekeeping to have the same structure as in AutoFixture.
  2. Add automated deployment via AppVeyor.
  3. Push .NET Core support.
  4. Change the default namespace to start with Albedo like we did in AutoFixture.
  5. Preserve all the copyrights (as I don't know what to do with them 馃槦) and will add my name to the package authors :)

One more question - do you want to be a member of that organization? I'd be happy if you could - you could give valuable advises if there are some question. I don't know this library philosophy well, so I'll need your assistance. Everything else (release management) will be my responsibility.

Are you fine with such a plan?

@zvirja
Copy link
Member Author

zvirja commented Sep 15, 2017

@ploeh Did you have any chance to review my reply? 馃槉 It shouldn't take too much time to transfer the repo, unless you have concerns regarding this plan.

This will enable me to proceed further as a lot of things should be done..

@ploeh
Copy link
Collaborator

ploeh commented Sep 16, 2017

A just tried to perform the transfer, but I get this error message:

You don鈥檛 have the permission to create repositories on AlbedoOrg

@ploeh
Copy link
Collaborator

ploeh commented Sep 16, 2017

BTW, I'm not going to commit the .snk to the repository, as I strongly disagree with the choice to do so. I understand and accept that you plan to do so, but then the commit should be in your name.

Send me an email, and I'll transfer the file to you. From there, you can do with it as you wish.

@ploeh
Copy link
Collaborator

ploeh commented Sep 16, 2017

@zvirja, I've added you as an owner of the Albedo NuGet package.

@zvirja
Copy link
Member Author

zvirja commented Sep 16, 2017

Hi Mark!

I made you a member, now you should accept the invitation and after should be able to create a repo.

As for the key transfer, you should already have my email. But just in case: zvirja1@gmail.com.

Thank you for your time!

Would you also agree to me a member of maintainers group and help with questions if any? I don't know the library concepts a lot..

@ploeh
Copy link
Collaborator

ploeh commented Sep 16, 2017

Transfer done!

@zvirja
Copy link
Member Author

zvirja commented Sep 16, 2017

@ploeh Thank you!

One more question - do you want to be a member of that organization? I'd be happy if you could - you could give valuable advises if there are some question. I don't know this library philosophy well, so I'll need your assistance. Everything else (release management) will be my responsibility.

Could you also reply to this?

@ploeh
Copy link
Collaborator

ploeh commented Sep 16, 2017

I don't think there's going to be much activity related to this project, so I'll stay a member for now. I'll leave if it becomes too much, though.

Whether or not I'm a member, you can always mention me if you have a question.

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.

Is it .NET Core ready?
2 participants