Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Port to core #19

Closed
wants to merge 7 commits into from
Closed

Port to core #19

wants to merge 7 commits into from

Conversation

Jaxx-22
Copy link

@Jaxx-22 Jaxx-22 commented Mar 13, 2017

Port to NetStandard v1.6
This is backward compatible with .NetFramework v4.5 and up
Will work with .NetCore apps

@trailmax
Copy link
Contributor

Thank you for your effort. But you have renamed the folder for the project from Cake.SqlServer to SqlServer and it is now impossible to see what has been changed in diffs. Please rename the folder back to what it was.

And build if run .\build.ps1 -Target Build is broken for me locally:

SqlServer.xproj(8,3): error MSB4019: The imported project "C:\Program Files (x86)\MSBuild\Microsoft\VisualStudio\v10.0\DotNet\Microsoft.DotNet.Props" was not found. Confirm that the path in the declaration is correct, and that the file exists on disk.`

Also am I right in saying that bacpac functionality is not going to work on .NetStandard? Any reason for that?

@Jaxx-22
Copy link
Author

Jaxx-22 commented Mar 14, 2017

Sorry for the rename, slight overview on my end.

And for the bacpac functionality, the code is using the dependency "Microsoft.SqlServer.DacFx.x64" : "130.3485.1" does not have a net standard build to use. So in code we look at which framework we are using.

There is still more work to be done with getting built too. I will be coming back to later on this week.

<file src="Microsoft.*.dll" target="lib/net45"/>
<file src="LICENSE" />
</files>
<?xml version="1.0" encoding="utf-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Line endings changed? white-space? files look identical, yet diff shows 100% change

[assembly: AssemblyInformationalVersion("1.5.1-backpac.1+13.Branch.backpac.Sha.ea1d7b9246a302adb0f73eb9f55a8375480a503c")]
[assembly: AssemblyCopyright("Copyright (c) 2016 - 2016 AMV Software")]

//------------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above. White-space?

@@ -414,6 +414,7 @@ public static void RestoreSqlBackup(this ICakeContext context, String connection
/// });
/// </code>
/// </example>
#if NET451
Copy link
Contributor

Choose a reason for hiding this comment

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

#if NET451 should be above the XML comment, otherwise VS goes crazy

@@ -451,7 +452,8 @@ public static void CreateBacpacFile(this ICakeContext context, String connection
/// });
/// });
/// </code>
/// </example>
/// </example>
#if NET451
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above - #if should include xml-comment

@@ -156,6 +156,7 @@ internal static SqlConnection OpenSqlConnection(ICakeContext context, String con
}
catch (SqlException exception)
{
#if Net541
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it not be NET451?

@@ -164,6 +165,16 @@ internal static SqlConnection OpenSqlConnection(ICakeContext context, String con
var newException = new Exception(errorMessage, exception);
throw newException;
}
#else
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a difference with the block of code above. Is #if NET451 required here?

@@ -0,0 +1,8 @@
namespace Tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file required?

@@ -172,7 +172,7 @@ public void GetDatabaseName_Should_ReturnName()

private static string GetBackupFilePath(String filename = "multiFileBackup.bak")
{
return Directory.GetFiles(AppDomain.CurrentDomain.BaseDirectory, filename, SearchOption.AllDirectories).FirstOrDefault();
return Directory.GetFiles(Directory.GetCurrentDirectory(), filename, SearchOption.AllDirectories).FirstOrDefault();
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@trailmax
Copy link
Contributor

I see not much code changes are actually required to get to .NetStandard. The biggest problem is with DacFx dependency, but it caused trouble before - see #18. I'll see what can be done about this. Ability to work with bacpac is a pretty big thing for me and my team.

As for exception I get on build - this is due to lack of tooling installed - I need to update to the latest .Core tools, and even to VS2017.

Exception on the build server is for packing nuget - folder structure generated by the new msbuild is different to what it was before, so build,cake needs to be adjusted.

@trailmax
Copy link
Contributor

I'm about to go on holidays. So please excuse the lack of responsiveness for a week-or-two.

@trailmax
Copy link
Contributor

trailmax commented Aug 31, 2017

Closing due to lack of response

@trailmax trailmax closed this Aug 31, 2017
@trailmax trailmax mentioned this pull request Nov 3, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants