Skip to content
This repository has been archived by the owner on Jan 30, 2021. It is now read-only.

Create availability set if availability_set_name is specified #136

Merged
merged 4 commits into from
Feb 3, 2017
Merged

Create availability set if availability_set_name is specified #136

merged 4 commits into from
Feb 3, 2017

Conversation

codestothestars
Copy link
Contributor

Finished implementing what appeared to be unfinished support for creating and setting an availability set for the VM. Before this, if availability_set_name was specified, the plugin would actually log to the console implying that an availability set would be created, although it would not create one.

@azurecla
Copy link

Hi @Codethestars, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.azure.com.

TTYL, AZPRBOT;

@azurecla
Copy link

@codestothestars, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, AZPRBOT;

@devigned
Copy link
Member

devigned commented Feb 2, 2017

I'll take a look at adding this functionality in a separate PR if you don't want to update this (my fault -- it's been a while since I reviewed PRs out here).

Thank you for the contribution.

@codestothestars
Copy link
Contributor Author

Do you mean update this PR to resolve the merge conflict? If so I'd be happy to, so please let me know.

@devigned
Copy link
Member

devigned commented Feb 2, 2017

@codestothestars, yes. If you are interested in updating the PR to resolve the conflict, I'll gladly review it.

@codestothestars
Copy link
Contributor Author

@devigned Thanks, I'm showing no conflicts now. First time resolving a conflict on GitHub so I hope it looks clean on your end.

@codestothestars
Copy link
Contributor Author

I see now that it's showing two Merge commits. For some reason the first time I "resolved" the conflict, it still said there were conflicts, even though the "Files changed" showed everything looking as it was supposed to. So I went through the same "resolve" steps a second time, and only then did it show "no conflicts". That must be why it shows two Merge commits, but I don't know why I had to do it a second time at all.

@devigned
Copy link
Member

devigned commented Feb 3, 2017

@codestothestars I'll give this a review this weekend.

If you'd like to learn how to make your PRs look better (no merge commits, restructured and/or reworded), check out https://www.atlassian.com/git/tutorials/rewriting-history (or more generally git-rebase). It's super helpful.

@devigned
Copy link
Member

devigned commented Feb 3, 2017

Actually, it looks good. I'm going to pull this in. It will land in 2.0.0.pre3.

@devigned devigned merged commit 8dfb41f into Azure:v2.0 Feb 3, 2017
@codestothestars
Copy link
Contributor Author

@devigned Great! I do want ideal PRs, so thanks for the resource.

@codestothestars codestothestars deleted the availability-set-name branch February 3, 2017 17:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants