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

Configuration for Windows and netcore Preview4 fsproj #4

Merged
merged 10 commits into from
Dec 9, 2016

Conversation

cloudRoutine
Copy link
Contributor

  • fixes build issues
  • upgrade code to work with newer package versions
  • use netcore fsproj format to build base Expecto lib
  • configure project with paket4
  • added sln for easy VS usage
  • started devguide
  • setup FAKE build for one command build.cmd repo setup
  • setup appveyor CI

@haf I really like this library and I'd like to help you make Expecto the first choice for F# testing

there are a few things we could do next -

  • setup netcore .fsproj for the other projects
  • create an expecto dotnet cli tool
  • expecto test runner for VS
  • layout roadmap for future functionality
  • expecto FAKE helper module

but these are just ideas, w/e you think should be next is cool with me 😸

Copy link
Contributor

@enricosada enricosada left a comment

Choose a reason for hiding this comment

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

  • pin to specific sdk version, otherwise it's going to be flacky
  • cleanups

build_script:

# .NET Core SDK binaries
- ps: $url = "https://dotnetcli.blob.core.windows.net/dotnet/Sdk/rel-1.0.0/dotnet-dev-win-x64.latest.zip"
Copy link
Contributor

Choose a reason for hiding this comment

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

set the url to download a specific version, is very very unstable to use dev latest like that.
also is not going to download preview3 atm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you keep saying that, but i've never had an issue with it

Copy link
Contributor

Choose a reason for hiding this comment

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

because i used that lot a long time, so i try to avoid pain to others. but as all suggestion, is just a suggestion.
It's the same reason why we use paket to pin dependencies version with lock file, reproducible build environment

{
"sdk": {
// "version": "1.0.0-preview4-004130"
"version": "1.0.0-preview4-004079"
Copy link
Contributor

Choose a reason for hiding this comment

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

global.json is deprecated and useless with msbuild based.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The global.json file is still present in the .NET Core Command Line Preview 3. However, its main purpose is not to define solution metadata as in previous releases, but to allow selection of the CLI version being used through the sdk property. +

This reference reflects the above fact.

https://docs.microsoft.com/en-us/dotnet/articles/core/preview3/tools/global-json

Copy link
Contributor

Choose a reason for hiding this comment

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

ah nice! thx @cloudRoutine

<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" />
<PropertyGroup>
<AssemblyName>Expecto.netcore</AssemblyName>
Copy link
Contributor

Choose a reason for hiding this comment

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

assembly name is Expecto so is easier to merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops missed that, I had to redo the migration from scratch so it got overlooked

<Version>1.0.0-alpha-20161104-2</Version>
<PrivateAssets>All</PrivateAssets>
</PackageReference>
<PackageReference Include="FSharp.Core">
Copy link
Contributor

Choose a reason for hiding this comment

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

you can write Version as attribute, so

 <PackageReference Include="FSharp.Core" Version="4.0.1.7-alpha" /> 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not doing that



</ItemGroup>
<ItemGroup Condition=" '$(TargetFramework)' == 'netstandard1.6' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to split in another item group, there is only netstandard as target framework, merge with previous ItemGroup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there's going to be other frameworks for crossgen build eventually

<Version>1.6.1</Version>
</PackageReference>
<PackageReference Include="FSharp.NET.Sdk">
<Version>1.0.0-alpha-000009</Version>
Copy link
Contributor

Choose a reason for hiding this comment

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

use 1.0.0-alpha-*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

last time you said to always use a specific version instead of *

<TargetFramework>netstandard1.6</TargetFramework>
<DebugType>pdbonly</DebugType>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)' == 'Release' ">
Copy link
Contributor

Choose a reason for hiding this comment

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

it's useles, is already defined in msbuild for Debug => DEBUG, Release => RELEASE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

generated by dotnet migrate, you should ask them why their tool makes useless properties 😜

Copy link
Contributor

Choose a reason for hiding this comment

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

i'll open an issue

<Compile Include="..\paket-files\logary\logary\src\Logary.Facade\Facade.fs" />
<Compile Include="..\Expecto\Expecto.fs" />
<Compile Include="..\Expecto\Expect.fs" />
<EmbeddedResource Include="**\*.resx" />
Copy link
Contributor

Choose a reason for hiding this comment

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

if no resx, just remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

i leave that in templates to make it easier for ppl to know there is that.
But maybe i should remove that, dunno what's the best.

@haf
Copy link
Owner

haf commented Dec 1, 2016

This is great! The only reason it's currently failing is that you're not replacing Logary.Facade with the Expecto.Logging namespace in Facade.fs.

@cloudRoutine
Copy link
Contributor Author

How should I do that? I changed it to Logary so I could get it to build initially on my machine

@haf
Copy link
Owner

haf commented Dec 1, 2016

Well, I use ruby, but I hear sed is up for the job too. Depends on your build environment: https://github.com/logary/logary/#using-logary-in-a-library

@cloudRoutine
Copy link
Contributor Author

ohhh, i thought there might be some metadata attribute or some aliasing trick I didn't know about

@cloudRoutine
Copy link
Contributor Author

@haf I don't think albacore knows how to handle paket groups properly, it's getting confused and adding the wrong nupkg to the generated nuspecs.

also is there a particular reason you want to use F# 3.1 over 4.0?

@haf
Copy link
Owner

haf commented Dec 1, 2016

@cloudRoutine Yes, it gets confused – the groups were added in after I wrote the parser for the lock files... As you can see, it's a really simple parser ;) https://github.com/Albacore/albacore/blob/master/lib/albacore/paket.rb

I don't mind running FAKE if you prefer to build nugets based on that. As long as I can do ./build.sh release as long as: versioning with assembly info, extracting all dependencies from the paket.references files automatically, building the changelog from the git commit messages, validating the tag doesn't exist in git, validating there are no dirty files in the current directory, tagging the new release in git, updating nuget.org, pushing to git, pushing git tags and handling graceful revert on failure as well as retries on network errors just work, like they do in albacore. It needs to be completely friction free to release nugets. Or we can update the albacore parser to support groups; your choice.

As for the F# versions; 3.1 was more compatible across dependent libraries, but I've upgraded almost all my own libraries to 4.0 so I don't mind upgrading this lib either.

@cloudRoutine
Copy link
Contributor Author

I've setup FAKE builds to do almost all of that in the past 😉, just not the reverts and I usually prefer to write the changelogs myself.

I'm not saying that you should switch from using it either, all I want is to leave FAKE in on the side, since that makes it easier to work with ionide and VS and it's the most familiar thing for most F# devs.

I brought up the FSharp.Core version since I know you want to take this to netcore and that's only going to be F# 4.1 and the .net standard library is going to be aligned against netframework 4.6.1, so you can stick with the old version and throw #directives all over the place, or upgrade and have a codebase that works without any changes.

@cloudRoutine
Copy link
Contributor Author

@haf this is the parser paket uses for the dependencies file
https://github.com/fsprojects/Paket/blob/master/src/Paket.Core/DependenciesFile.fs

@haf
Copy link
Owner

haf commented Dec 7, 2016

@cloudRoutine Just peeking in here to ask whether there's something you're awaiting me to complete? Also, I don't mind giving push access to this repo for you both – what do you say?

@cloudRoutine
Copy link
Contributor Author

i thought you wanted to fix the parser for albacore before merging

@haf
Copy link
Owner

haf commented Dec 7, 2016

@cloudRoutine Well I don't have time to do that right now, because I'm in the middle of a feature with my team at qvitoo as well as having just published the C# facade in Logary and working towards fully-fledged metrics in Logary as well. (long sentence there...)

But I don't mind hacking the Rakefile when I need to make next release; that's quick for me to do.

@cloudRoutine
Copy link
Contributor Author

sounds good then, I think this is ready to merge. I'm fine with having push access, but I'll still generally add big code changes through PRs. Docs and minor bugfixes are usually all I update directly on the repos I work on with other people (Ionide, VFPT, etc).

@haf
Copy link
Owner

haf commented Dec 8, 2016

Now we have a prerelease dependency, so the build fails. Could you please port https://github.com/logary/logary/blob/master/Rakefile#L111 to this repo?

@cloudRoutine
Copy link
Contributor Author

The prerelease dependency causing the build to fail is because the parser isn't handling the groups properly. If it was doing a paket restore just on the group main and only packaging the normal projects and not the netcore projects this wouldn't be an issue.

It wasn't my intention for the netcore build to be released in the nupkg yet.

@haf
Copy link
Owner

haf commented Dec 8, 2016

Hmm, ok. I'll let you decide what to do: merge and get core support but a broken build (albeit at the very last step to create packages) or to await one of us (likely me) fixing the parser?

@cloudRoutine cloudRoutine force-pushed the configuration branch 2 times, most recently from 2dce1f4 to f93c7ea Compare December 9, 2016 01:20
@cloudRoutine
Copy link
Contributor Author

or I'll tweak the rake file so it ignores the netcore dir 😉

@haf
Copy link
Owner

haf commented Dec 12, 2016

I've created an issue in albacore for this Albacore/albacore#221 – it's marked "help wanted" for now.

@haf haf mentioned this pull request Dec 19, 2016
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.

3 participants