patternjavascriptMinor
Cardshifter web client deploy script
Viewed 0 times
scriptcardshifterdeployclientweb
Problem
The Cardshifter web client is coming along! The game client is a web app written mostly in Angular.js and is hosted at at play.cardshifter.com.
@SimonForsberg (who runs the server) is getting tired of the web people always working tirelessly and updating the code, and asked for a script that can do deployment for him.
Requirements
Background
I'm quite new to JavaScript and Node.js land. I chose to still use it for three reasons:
I chose to pass credentials through environment variables because it looks like Travis CI handles encrypted variables well. A person that has the credentials can also set up a simple script to run the deployment with those specific variables.
@skiwi told me about promises after having written the code originally, but it prompted me to convert the code to use them instead of normal callbacks. I think this lead to a great improvement of the readability of the code.
Simon hasn't told me the address of the FTP server yet, which is why I left
``
* 4. Profit!
*
* Environment variables used:
* - DEPLOY_FTP_USERNAME: Username used to log in through FTP.
* - DEPLOY_FTP_PASSWORD: Password used to log in through FTP.
* - DEPLOY_DUGA_
@SimonForsberg (who runs the server) is getting tired of the web people always working tirelessly and updating the code, and asked for a script that can do deployment for him.
Requirements
- Upload the contents of
dist/to/assets/andwww/to/through FTP.
- Post a message to the chat bot Duga using a HTTP request when done.
- Can be run automatically on Travis CI.
Background
I'm quite new to JavaScript and Node.js land. I chose to still use it for three reasons:
- Dependency management already set up with Npm
- Project is already JavaScript; easier maintenance
- Fun to learn :)
I chose to pass credentials through environment variables because it looks like Travis CI handles encrypted variables well. A person that has the credentials can also set up a simple script to run the deployment with those specific variables.
@skiwi told me about promises after having written the code originally, but it prompted me to convert the code to use them instead of normal callbacks. I think this lead to a great improvement of the readability of the code.
ftp-deploy was acting up when uploading dist/ and www/ separately, which is why the script is now first copying everything to a temporary directory with a structure emulating the server. One problem was that if remoteRoot didn't exist, the upload would fail.Simon hasn't told me the address of the FTP server yet, which is why I left
localhost:2121 in there.``
/* Deploy the client to play.cardshifter.com
*
* How to use:
* 1. Build the project and make sure that everything is in order.
* 2. Set up environment variables (see below).
* 3. Run npm run deploy`.* 4. Profit!
*
* Environment variables used:
* - DEPLOY_FTP_USERNAME: Username used to log in through FTP.
* - DEPLOY_FTP_PASSWORD: Password used to log in through FTP.
* - DEPLOY_DUGA_
Solution
There's a few points you could improve on, but for the most part, your code looks good.
Funny memes are funny, but, don't belong in comments :-(
On a related note, this kinda thing probably belongs in one of the readme.md files, or a GitHub wiki page for the web client.
You can actually omit the
Which can become:
Although, this can be dangerous if you forget the commas, as they can rise through the function to find a higher-level variable declaration, and become global in the process. You can read about that here.
You shouldn't have random magic numbers/spells everywhere, use a spellbook (define them) instead.
Or, make them parameters for input.
Remember that the chatrooms you're sending messages to support Markdown, it's best to use it.
into:
Consider using
Do you need to include the
Probably not.
The following could be a ternary:
which can turn into:
Funny memes are funny, but, don't belong in comments :-(
* How to use:
* 1. Build the project and make sure that everything is in order.
* 2. Set up environment variables (see below).
* 3. Run `npm run deploy`.
* 4. Profit!On a related note, this kinda thing probably belongs in one of the readme.md files, or a GitHub wiki page for the web client.
You can actually omit the
vars when you declare a few at once:var copy = require('recursive-copy');
var FtpDeploy = require('ftp-deploy');
var path = require('path');
var request = require('request-promise');
var temp = require('temp').track();Which can become:
var copy = require('recursive-copy'),
FtpDeploy = require('ftp-deploy'),
path = require('path'),
request = require('request-promise'),
temp = require('temp').track();Although, this can be dangerous if you forget the commas, as they can rise through the function to find a higher-level variable declaration, and become global in the process. You can read about that here.
You shouldn't have random magic numbers/spells everywhere, use a spellbook (define them) instead.
host: "localhost",
port: 2121
roomId: 16134,Or, make them parameters for input.
Remember that the chatrooms you're sending messages to support Markdown, it's best to use it.
text: "New web client version uploaded to http://play.cardshifter.com/."into:
text: "New web client version uploaded to [play.cardshifter.com](http://play.cardshifter.com/)."Consider using
.joins in future, also, JavaScript's string concatenation is a bit slow. Array joins run a bit faster, and let you use specified 'glue' also.console.log("Deploying to ftp://" + config.host + ":" + config.port + "...");Do you need to include the
Content-Length header in the API request?Probably not.
config.headers["Content-Length"] = json.length;The following could be a ternary:
if (err) {
reject(err);
} else {
resolve();
}which can turn into:
err ? reject(err) : resolve();Code Snippets
* How to use:
* 1. Build the project and make sure that everything is in order.
* 2. Set up environment variables (see below).
* 3. Run `npm run deploy`.
* 4. Profit!var copy = require('recursive-copy');
var FtpDeploy = require('ftp-deploy');
var path = require('path');
var request = require('request-promise');
var temp = require('temp').track();var copy = require('recursive-copy'),
FtpDeploy = require('ftp-deploy'),
path = require('path'),
request = require('request-promise'),
temp = require('temp').track();host: "localhost",
port: 2121
roomId: 16134,text: "New web client version uploaded to http://play.cardshifter.com/."Context
StackExchange Code Review Q#104627, answer score: 5
Revisions (0)
No revisions yet.