HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Cardshifter web client deploy script

Submitted by: @import:stackexchange-codereview··
0
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

  • Upload the contents of dist/ to /assets/ and www/ 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 :-(

* 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.