-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Deploy to Azure button #124
Conversation
@@ -11,7 +11,7 @@ | |||
"express": "4.11.0", | |||
"hostenv": "1.0.1", | |||
"opentype.js": "0.4.4", | |||
"socket.io": "1.3.5", | |||
"socket.io": "1.3.6", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.3.6 addressed a build issue on Windows.
Pushed |
Rather than fixing it to a particular tag, the button will deploy the latest version that is available on npm (which is 0.7.3 right now). |
Is there a free tier like Heroku? |
There is a free tier for web apps (PaaS). However, as slackin uses native node modules, we are forced to use virtual machines which does not have a free tier. New Azure signups have $200 credit which would pay for running a slackin instance deployed through this button for about 16 months. |
We don't depend on them. Any way to force the free tier? |
Looks like I'm mistaken. Web apps run node-gyp just fine. However, I think we'll need to tinker with the build as Windows doesn't come preinstalled with gnu make. So either we download/install gnu make on the web app (not even sure if the sandbox will allow me to execute a random exe) or replace the makefile with gulp + babel. What do you think? Is there a reason why we are using make? |
Totally down for ditching |
👍 I'll update the PR (hopefully over the weekend) |
Will re-open PR with new changes in a bit. |
Here's what it looks like so far with the following implemented:
|
wow |
this would be awesome. I love reproducible and predictable builds. |
I hope that's a good wow 😏 |
It was an amazing wow |
@jpoon can we also get a retina button (SVG)? The button above looks blurry |
They don't seem to have one at the moment: projectkudu/slingshot#28. 😞. In the meantime, as suggested by the bug, we can use a shields.io one. |
🎉 All done. If you press the deploy button, it'll bring you to this screen: You can choose what Slackin Release you want:
After going through the deployment wizard, it'll bring you to this screen: I manually verified that both stable and latest work. Having deployed our VsCodeVim slackin instance via the Azure button. 😄 Sidenote: As |
Super awesome work. |
}); | ||
|
||
gulp.task('clean', function() { | ||
return gulp.src(paths.out_js, { read: false }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The indentation doesn't seem to match the project's?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Addressed comments and squashed commits. |
My only oustanding concern is how many files we have now for cloud deployment in source control. After this deploy we'll end up with:
in order to support Docker, Heroku and Azure. |
For each cloud provider, you'd need to define a minimum set of deploy/setup logic regardless. In the case of Azure, I did my best in limiting the amount of files that I need to touch. Edit: Actually, I might be able to eliminate one file. Testing out my theory. Will update in a sec. |
@@ -29,7 +34,7 @@ | |||
}, | |||
"scripts": { | |||
"test": "mocha", | |||
"postinstall": "make", | |||
"start": "./bin/slackin $SLACK_SUBDOMAIN $SLACK_API_TOKEN" | |||
"postinstall": "./node_modules/.bin/gulp", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can just write gulp
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That assumes it is installed globally which may not be the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought NPM aliased the local binaries as globals for the script declarations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL. Fixed.
.pipe(rimraf()); | ||
}); | ||
|
||
gulp.task('default', ['es6to5', 'copyassets']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest of the repo uses ES6. Sticks out a bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needed to add a .babelrc
but this is now es6.
@jpoon some more comments. Sorry about the extensive review. |
re: my comment about the number of files. I was simultaneously ranting about the inherent tradeoff between supporting many cloud providers out-of-the-box and the cleanliness of the project root. Things like The fewer the files and the more succinct they are, the better for the project. |
Removed
Yeah, I feel ya... not sure what can be done though. |
@rauchg anything else to address to get this merged? To clean up the root path, we could create a
Everything else (incl. Heroku Procfile) still needs to be in the root. |
Maybe |
@rauchg done! Also:
|
* Uses ARM template to create a Web App (Free Tier) * When deploying, choose between `Latest` (deploys code from git repo) or `Stable` (deploy from latest published npm package). * Server will be accessible at `<sitename>.azurewebsites.net`
.pipe(rimraf()); | ||
}); | ||
|
||
gulp.task('default', ['es6to5', 'copyassets']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is great, thank you!
What it looks like (feel free to click the button below, it works 🎉 ):
Uses ARM template to create a Standard_A0 sized VM (smallest/cheapest option). Upon startup, the VM will install Docker and run the slackin image. The slackin server will be reachable at
{whatever-you-configured-as-your-dns-in-the-setup}.westus.cloudapp.azure.com
.Re: #96