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

xSQLServerDatabase: Changed SQLInstance to SQLInstanceName #579

Merged
merged 4 commits into from
Jun 4, 2017
Merged

xSQLServerDatabase: Changed SQLInstance to SQLInstanceName #579

merged 4 commits into from
Jun 4, 2017

Conversation

Blankf
Copy link
Contributor

@Blankf Blankf commented May 26, 2017

Changed the documenation for the xSQLServerDatabase, there was an error in there because the sqlinstance should be sqlinstancename.

checked the code for this.


This change is Reviewable

@msftclas
Copy link

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@codecov-io
Copy link

codecov-io commented May 26, 2017

Codecov Report

Merging #579 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##            dev   #579   +/-   ##
===================================
  Coverage    81%    81%           
===================================
  Files        32     32           
  Lines      3434   3434           
===================================
  Hits       2788   2788           
  Misses      646    646

@johlju
Copy link
Member

johlju commented May 26, 2017

@Blankf Thank you for this PR!

Even if this is an small change, could you please update the Unreleased section of the file CHANGELOG.md what changed?

@johlju johlju added waiting for author response The pull request is waiting for the author to respond to comments in the pull request. needs review The pull request needs a code review. and removed waiting for author response The pull request is waiting for the author to respond to comments in the pull request. labels May 26, 2017
@johlju
Copy link
Member

johlju commented May 27, 2017

Thanks for updating the CHANGELOG.md. Although since I merged another PR you need to rebase this PR so those new merged changes get into this PR.
Normally I can do this from GitHub directly, but don't get that option this time for some reason.

So could you rebase your PR? You an read how that is done here https://github.com/PowerShell/DscResources/blob/master/GettingStartedWithGitHub.md#resolve-merge-conflicts
Let me know if you have any problems, and I try to help!

Or the other alternative is to check "Allow edit from maintainers" for this PR, to the right here, under Notifications. Then I can help you rebase it and push it directly into your branch, which will update this PR.

@johlju johlju added waiting for author response The pull request is waiting for the author to respond to comments in the pull request. and removed needs review The pull request needs a code review. labels May 27, 2017
@Blankf
Copy link
Contributor Author

Blankf commented May 30, 2017

i've did a rebase, that went fine but it still gives an error here. i've ticket 'Allow edits from maintainers'
so if you could help me out, thnx!

@johlju
Copy link
Member

johlju commented May 30, 2017

Seems that you got a lot of old commits into this one now. So something went wrong. Might be because you working in the dev branch, so the steps is a little different then.
If you want to try again, here is what I would do

I assume 'origin' below is the remote name to PowerShell repository, if not, change appropriately (you can list remote names with git remote -v).
And I assume 'my' is the remote name that points to your fork of xSQLServer.

git checkout dev # your working branch
git fetch origin dev # fetch changes from PowerShell/xSQLServer/dev
git rebase origin/dev
# resolve any conflicts (if any); save any changed files, 'add file' for each file to stage (or just click the + sign in VSCode when file has been changed and saved), then run 'git rebase --continue'. 
git push my dev --force

If you haven't had the time I am happy to help you resolve this, when I get home later tonight. :)

@johlju
Copy link
Member

johlju commented Jun 2, 2017

Seems the rebase went fine, but the conflicts were not resolve correctly, but instead ended up with the conflicts in the change log. But that can be fixed. And we did a new release also, so it needs to be rebased again because of that.

See here 68b86c5

You see this headers

<<<<<<< HEAD
Some text
=======
some text
>>>>>>> added information to the changelog

This block needs to be manually resolved, saved and then staged before running git rebase --continue.

To fix, you need to rebase again, and resolve the conflict (you must edit the file manually at the step # resolve any conflicts).

To make matters a bit more complicated, we just release a new version which means when rebasing, git does not understand that we move the change history (unreleased section) to a new version section. So you need to move your lines back up to the unreleased section.
If there is any problem after rebasing, then if you like, you can just correct any problems by pushing another commit of the CHANGELOG.md.

Do you want to give this another try? Otherwise I will help you as soon as I have time, pro baby after the weekend (been more busy lately than I thought I would be).

@johlju
Copy link
Member

johlju commented Jun 4, 2017

@Blankf I rebased this one, and pushed a new commit to solve the change log (it could not be done in rebase).

@johlju
Copy link
Member

johlju commented Jun 4, 2017

:lgtm:


Reviewed 1 of 1 files at r1, 1 of 1 files at r3, 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@johlju
Copy link
Member

johlju commented Jun 4, 2017

@Blankf Again, thank your for sending in this fix! 😄 I will merge this one as soon as the tests complete.

@johlju johlju changed the title xSQLServerDatabase: Changed Sqlinstance to Sqlinstancename xSQLServerDatabase: Changed SQLInstance to SQLInstanceName Jun 4, 2017
@johlju johlju removed the waiting for author response The pull request is waiting for the author to respond to comments in the pull request. label Jun 4, 2017
@johlju johlju merged commit 15b873f into dsccommunity:dev Jun 4, 2017
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.

4 participants