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

Merge Management.Sql backport branch into mainline branch #4262

Closed
wants to merge 2 commits into from

Conversation

jaredmoo
Copy link
Contributor

@jaredmoo jaredmoo commented Apr 24, 2018

Description

We recently added support for SQL Managed Instance, Managed Instance database, and Managed Instance failover groups to the mainline psSdkJson6 branch in PR #4244 and released as 1.15.0-preview.

We then realised that we wanted to backport this change to before the 1.14.0-preview breaking changes, so in PR #4257 we cherry picked these changes into the new backport branch 'ManagementSql-CloudLifter' and released as 1.13.1-preview.

In order to reconcile these cherry picks and keep history healthy moving forward, we need to merge from the backport branch (ManagementSql-CloudLifter) into the mainline branch (psSdkJson6). This PR does that. There are no files changed, just merged from ManagementSql-CloudLifter into psSdkJson6 and then resolved conflicts.


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

ziwa-msft and others added 2 commits April 23, 2018 16:29
…h Instance Failover Group APIs (Azure#4259)

* adding generated files and tests for Cloud Lifter work

* updating the SDK version in .csproj and AssemblyInfo.cs files

* adding Managed Instance test recording

* addressing some comments

* changing tabs into spaces

* Regen from swagger

* Updating the release note with the current changes
@jaredmoo
Copy link
Contributor Author

@ziwa-msft and @shahabhijeet FYI. This is not urgent (does not block any release).

@shahabhijeet
Copy link
Member

I don't quite follow.
What are we merging if there are no files?

@jaredmoo
Copy link
Contributor Author

There are no files changed, but there are merge conflicts that were resolved. All of them were resolved by 'keep ours', i.e. keep the psSdkJson6 change (not SqlMgmt-CloudLifter).

@shahabhijeet
Copy link
Member

I still don't know why you would like to merge it. What happens if you don't merge it?
because you are honoring all the files from latest psSdkJson6, so does it really matter if you are mege?

@jaredmoo
Copy link
Contributor Author

Cherry picking in general is bad (Raymond Chen wrote a fascinating series of articles about it: https://blogs.msdn.microsoft.com/oldnewthing/20180323-01/?p=98325 ). There will soon be another PR for short-term retention going into SqlMgmt-CloudLifter branch, and when that PR lands we want to merge it into psSdkJson6 so that both branches have the same changes for the new feature. Merging this now resolved conflicts and will make that future merge easier.

If you want, we can hold off on this for now until that short term retention change lands in SqlMgmt-CloudLifter.

@shahabhijeet
Copy link
Member

yes let's hold off on this one.

@jaredmoo jaredmoo closed this Apr 24, 2018
@jaredmoo
Copy link
Contributor Author

No problem.

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