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

Add support for publish profiles #2366

Merged
merged 6 commits into from Dec 17, 2019

Conversation

@kmadof
Copy link

kmadof commented Aug 7, 2019

Description

I added support for publish profile file. I changed the way how we set default values for deployment arguments, because arguments given from command line override values from publish profile. So when publish profile is given we must set command lin only when they explicitly set, and not when they are not given like it was before. When publish profile is not given I set command line arguments like it was before.

If available, link to an existing issue this PR fixes. For example:

@kmadof

This comment has been minimized.

Copy link
Author

kmadof commented Aug 7, 2019

The build failure appears to be not related to my changes.

@matthid

This comment has been minimized.

Copy link
Collaborator

matthid commented Aug 7, 2019

Thanks for taking care of this! As this is basically a breaking change we need to think about how we want to handle it...We can:

  • remove the breaking change
  • add new stuff and obsolete the old stuff
  • split the package out of this repository and increase the major version
  • something different?
@kmadof

This comment has been minimized.

Copy link
Author

kmadof commented Aug 8, 2019

To remove the breaking change I would have to use nullable types instead of option, but this is not best idea in F# I think.
I'm not familiar with spliting the package out of repository, so I would try first the second option. It will take probably few days.

@github-actions

This comment has been minimized.

Copy link

github-actions bot commented Nov 30, 2019

There has not been any activity in this pull request for the last 3 months so it will be closed in 14 days if there is no activity.

@github-actions github-actions bot added the stale label Nov 30, 2019
@kmadof

This comment has been minimized.

Copy link
Author

kmadof commented Dec 10, 2019

Sorry for such long break. I want to make a new module SqlPackage and put there my changes, leaving old DacPac module without change. @matthid would be better to revert these commits and make change here in this PR, or maybe create new branch and new PR?

@matthid

This comment has been minimized.

Copy link
Collaborator

matthid commented Dec 10, 2019

@kmadof Sure just push ahead or create a new PR whatever you prefer.

@kmadof

This comment has been minimized.

Copy link
Author

kmadof commented Dec 11, 2019

@matthid can we remove stale label to be sure that PR won't be closed.

@kmadof

This comment has been minimized.

Copy link
Author

kmadof commented Dec 11, 2019

@matthid can you help me find out what I did wrong? I really don't understand how my changes impacted DotNet Core Integration tests.

@matthid matthid removed the stale label Dec 11, 2019
@matthid

This comment has been minimized.

Copy link
Collaborator

matthid commented Dec 11, 2019

@kmadof They didn't, I guess we need to update paket due to fsprojects/Paket#3743. I'll try to get the release/next branch fixed

@kmadof

This comment has been minimized.

Copy link
Author

kmadof commented Dec 15, 2019

@matthid did you have time to fix release/next?

@matthid

This comment has been minimized.

Copy link
Collaborator

matthid commented Dec 15, 2019

Yes, only travis is still doing something weird but it is quite tricky to debug as it happens only on travis

@kmadof

This comment has been minimized.

Copy link
Author

kmadof commented Dec 17, 2019

@matthid I see that now all checks passed. Cool. Thanks for that. What's next?

@matthid

This comment has been minimized.

Copy link
Collaborator

matthid commented Dec 17, 2019

Sorry for the late review, I asked two small questions and I think one small thing is missing. You added a new documentation file (help/markdown/sql-sqlpackage.md) which is awesome, but for someone to find it you should add links somewhere (probably in the menu in help/templates/template.cshtml)

Otherwise, the approach looks good to me.

@matthid matthid merged commit 990cc69 into fsharp:release/next Dec 17, 2019
4 checks passed
4 checks passed
greeting
Details
FAKE-CI #1444 succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@matthid matthid mentioned this pull request Dec 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.