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

Adds mlnet src and test projects from feature branch #4496

Open
wants to merge 5 commits into
base: master
from

Conversation

@maryamariyan
Copy link
Member

maryamariyan commented Nov 22, 2019

Adds mlnet src and test projects from features/automl into master branch

  • pack mlnet

cc: @LittleLittleCloud @eerhardt

@maryamariyan maryamariyan requested a review from dotnet/mlnet-core as a code owner Nov 22, 2019
@maryamariyan maryamariyan requested a review from dotnet/mlnet-automl as a code owner Nov 22, 2019
@maryamariyan maryamariyan force-pushed the maryamariyan:add-mlnet branch 3 times, most recently from da1be5a to fdc8c2f Nov 22, 2019

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>netcoreapp2.1</TargetFramework>

This comment has been minimized.

Copy link
@LittleLittleCloud

LittleLittleCloud Nov 25, 2019

Contributor

Can we upgrade this to net core 3.0? it should be simply a one - line change @maryamariyan @JakeRadMSFT @eerhardt

I know that the current recommend way from dotnet is to downgrade to netcore 2.1. But we continuously received issues from customers complaining about cli can't run on netcore 3.0. Even after hint message about downgrading was shown.

This comment has been minimized.

Copy link
@eerhardt

eerhardt Dec 2, 2019

Member

Let's do that in a separate change from porting the code. Can you please log an issue for it?

The drawback is that once you upgrade to 3.0, you are requiring users to install 3.0. But 2.1 is the current long-term support version.


using System.Runtime.CompilerServices;

[assembly: InternalsVisibleTo("mlnet.Tests, PublicKey=002400000480000094000000060200000024000052534131000400000100010015c01ae1f50e8cc09ba9eac9147cf8fd9fce2cfe9f8dce4f7301c4132ca9fb50ce8cbf1df4dc18dd4d210e4345c744ecb3365ed327efdbc52603faa5e21daa11234c8c4a73e51f03bf192544581ebe107adee3a34928e39d04e524a9ce729d5090bfd7dad9d10c722c0def9ccc08ff0a03790e48bcd1f9b6c476063e1966a1c4")]

This comment has been minimized.

Copy link
@LittleLittleCloud

LittleLittleCloud Nov 25, 2019

Contributor

Do we need to do this explicitly after merging to master?

<PackageReference Include="Microsoft.DotNet.PlatformAbstractions" Version="$(MicrosoftDotNetPlatformAbstractionsVersion)" />
  </ItemGroup>

  <Target Name="CreateManifestResourceNames" />

This comment has been minimized.

Copy link
@LittleLittleCloud

LittleLittleCloud Nov 25, 2019

Contributor

What's this empty task for...

This comment has been minimized.

Copy link
@eerhardt

eerhardt Dec 2, 2019

Member

It is required by the Common targets in MSBuild, but not used here.

See https://github.com/search?p=2&q=org%3Adotnet+CreateManifestResourceNames&type=Code for others doing the same.

Let's add a comment here, like in the other places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.