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

Location fix #1727

Merged
merged 2 commits into from
Jun 11, 2016
Merged

Location fix #1727

merged 2 commits into from
Jun 11, 2016

Conversation

marcvaneijk
Copy link
Contributor

Changed resource location from parameter to resourceGroup().location.
Deleted location parameter from deployment template, nested templates
and parameter file.

Changed resource location from parameter to resourceGroup().location.
Deleted location parameter from deployment template, nested templates
and parameter file.
@singhkays
Copy link
Contributor

@netwmr01 Can you review this change and see if you need to have a different location for resources than the resource group in your template?

@netwmr01
Copy link
Contributor

we don't need to have different resources in different location as the parent resources group. That being said, why the change and why now?

@singhkays
Copy link
Contributor

@netwmr01 We recently created a "Best Practices" guide for templates https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/best-practices.md

Now we're trying to make the existing templates consistent with those so that any future templates that start off from existing templates at least have a base of "best practices" in place.

@marcvaneijk marcvaneijk mentioned this pull request Apr 21, 2016
@netwmr01
Copy link
Contributor

@singhkay did this change pass the Travis CI build?

@singhkays
Copy link
Contributor

@marcvaneijk Can you test this out locally?

@marcvaneijk marcvaneijk merged commit 1ae7981 into Azure:master Jun 11, 2016
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