Skip to content

Conversation

@eladiw
Copy link
Contributor

@eladiw eladiw commented Apr 6, 2020

Supporting Windows and Linux web apps. Linux by default

Note to reviewers, please try to deploy using two options (win and linux).

Copy link

@tamirkamara tamirkamara left a comment

Choose a reason for hiding this comment

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

I've tested both options and it works.
I think we can add some directions on the main readme with regards to the options, something like - if you need online file editing in the portal please choose "windows", otherwise we recommend you go with the defaults (Linux).

tamirkamara
tamirkamara previously approved these changes Apr 6, 2020
azuredeploy.json Outdated
"windows"
],
"metadata": {
"description": "Host type: Linux or Windows"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can also state here in the description that linux is recommended?
something like "Host type: Linux or Windows (Linux is recommended)"
@GuyBeckerMicrosoft @tamirkamara

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Choose a reason for hiding this comment

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

BTW, I don't think this is displayed anywhere. Have you seen something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no. but it might be client related. maybe with CLI it does appear...

"workerSize": "0",
"workerSizeId": "0",
"hostingEnvironment": "",
"nodeVersion": "12.13.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use LTS instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh, I never found a complete list of possible values and I took this one from a template I generated from the portal.

README.md Outdated

[![Deploy to Azure](https://azuredeploy.net/deploybutton.png)](https://azuredeploy.net/)

Note: to enable online file editing in the portal please choose "Windows", otherwise we recommend you to go with the default (Linux)
Copy link
Contributor

Choose a reason for hiding this comment

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

@GuyBeckerMicrosoft do you have any comments on the wording? maybe using the teem "Azure Portal" instead of "portal", to not be confused with the bot portal

@amir-microsoft amir-microsoft merged commit 6c436a4 into microsoft:master Apr 6, 2020
amir-microsoft added a commit that referenced this pull request Apr 8, 2020
* Supporting Windows and Linux web apps. Linux by default (#57)

* update readme

Current locale validation doesn't support all cases, such as: de, zh-Hant-TW, En-au,aZ_cYrl-aZ
The locale is validated in the WebChat app anyway so it's redundant.
amir-microsoft added a commit that referenced this pull request Jun 15, 2020
* Supporting Windows and Linux web apps. Linux by default (#57)

* Supporting Windows and Linux web apps. Linux by default

* bug fix

* update readme

* adding doc

* update readme

* Origin/deployment script update (#64)

* Removing locale validation: (#63)

Current locale validation doesn't support all cases, such as: de, zh-Hant-TW, En-au,aZ_cYrl-aZ
The locale is validated in the WebChat app anyway so it's redundant.

* Dynamic branch and repo for deploy (#70)

* When using "deploy to azure" button, taking the repo and branch from the page where the button was clicked

* Adding example for using Content security policy to limit which domains are allowed to embed this page as a frame

* remove private preview from readme (#76)

* remove private preview from readme

* Description change

Co-authored-by: Guy Becker <gubecker@microsoft.com>

* Update index.html

* Fixing a bug where a redundant parameters was passed to the function passed to

Co-authored-by: Elad Iwanir <13205761+eladiw@users.noreply.github.com>
Co-authored-by: GuyBeckerMicrosoft <61974899+GuyBeckerMicrosoft@users.noreply.github.com>
Co-authored-by: Guy Becker <gubecker@microsoft.com>
Co-authored-by: AdamWalkerMicrosoft <40535367+AdamWalkerMicrosoft@users.noreply.github.com>
Co-authored-by: amir-microsoft <>
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