-
Notifications
You must be signed in to change notification settings - Fork 47
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
Verify that GenerateCreateScript works #599
Conversation
Hi @ErikEJ, I hope you don't mind, but I'm encountering some issues. I created a codespace with your branch and ran dotnet build on the test project, but I'm still seeing build errors. Could you help me figure out what I might be doing wrong? |
These are manual test, yes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What branch are you on, I couldn't see that from the screenshot. Looks like an older version somehow. We've had support for .NET 8 for quite a while now. |
Thanks! The verifies that GenerateCreateScript works with an external "master" dacpac reference. It did require setting If this was NOT set, the following error happended:
|
Wonder if I should update the section about system databases with this? <ItemGroup>
<PackageReference Include="Microsoft.SqlServer.Dacpacs.Master" Version="160.2.1" DacpacName="master" DatabaseVariableLiteralValue="master" />
</ItemGroup>
|
Yeah, I think that is a good idea. Kind of a weird exception though. |
@jmezach I updated the readme |
I guess this PR needs the same changes as #598 for the pipeline to work again. Might want to merge that one first and then rebase this on top. |
Yes, that was my plan... |
@jmezach Build unbroken 🎉 |
Nice. I think we can merge this. Doesn't really affect the product itself. |
Still waiting for the second approver... |
relates to #591