Skip to content

Conversation

@darrinhodges
Copy link
Contributor

Best Practice Checklist

Check these items before submitting a PR... See the Contribution Guide for the full detail: https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/README.md

  1. uri's compatible with all clouds (Stack, China, Government)
  2. Staged artifacts use _artifactsLocation & _artifactsLocationSasToken
  3. Use resourceGroup().location for resource locations
  4. Folder names for artifacts (nestedtemplates, scripts, DSC)
  5. Use literal values for apiVersion (no variables)
  6. Parameter files (GEN-UNIQUE for value generation and no "changemeplease" values
  7. $schema and other uris use https
  8. Use uniqueString() whenever possible to generate names for resources. While this is not required, it's one of the most common failure points in a deployment.
  9. Update the metadata.json with the current date

For details: https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/bp-checklist.md

  • - Please check this box once you've submitted the PR if you've read through the Contribution Guide and best practices checklist.

Changelog

  • Removed references to all non-Mahara code and references
  • Update stack image and documentation
  • Some URLs in the README.md will be incorrect until merged:
    • Deploy to Azure link
    • Visualize link
    • Issues link
    • Repo link

The point '"Has anyone run this template sucessfully in production? Yes they have. With that being said we do not make any performance guarantees about this architecture"' has been left in the README.md, should this be removed as part of the PR ?

The content URL for github has been left as the MS Azure links since the previous PR review suggested to update it as such. See #4283 (comment)

Does Mahara come under the banner of a 'managedApplication' or should that reference (and directory) be removed as part of the PR ?

Thanks

Darrin

darrinhodges added a commit to darrinhodges/azure-quickstart-templates that referenced this pull request Jul 20, 2018
darrinhodges added a commit to darrinhodges/azure-quickstart-templates that referenced this pull request Jul 20, 2018
"type": "Microsoft.Resources/deployments",
"apiVersion": "2017-05-10",
"apiVersion": "2016-09-01",
"name": "pid-738e3eec-68d4-4667-8377-c05c77c21f1b",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rgardler - is this correct?


grep "^//$storageAccountName.file.core.windows.net/mahara\s\s*/mahara\s\s*cifs" /etc/fstab
if [ $? != "0" ]; then
echo -e "\n//$storageAccountName.file.core.windows.net/mahara /mahara cifs credentials=/etc/mahara_azure_files.credential,uid=www-data,gid=www-data,nofail,vers=3.0,dir_mode=0770,file_mode=0660,serverino,mfsymlinks" >> /etc/fstab
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still needs to be addressed

{
"type": "Microsoft.Resources/deployments",
"apiVersion": "2016-09-01",
"name": "pid-738e3eec-68d4-4667-8377-c05c77c21f1b",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rgardler - is this correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not correct. This has been copied from a different template and should be removed from here. Thanks.

password=$storageAccountKey
EOF
chmod 600 /etc/mahara_azure_files.credential

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need these hardcoded endpoints fixed...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I raised an issue about this, please see #4960

@bmoore-msft
Copy link
Collaborator

@rgardler - one comment in here for you...

@bmoore-msft bmoore-msft merged commit ee29b0b into Azure:master Aug 4, 2018
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