11
\$\begingroup\$

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:

  1. Dependency management already set up with Npm
  2. Project is already JavaScript; easier maintenance
  3. 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_KEY: Duga API key. If not set the chat bot post is skipped.
 */
'use strict';

var copy = require('recursive-copy');
var FtpDeploy = require('ftp-deploy');
var path = require('path');
var request = require('request-promise');
var temp = require('temp').track();

function ftpConfig(local, remote) {
    return {
        username: process.env.DEPLOY_FTP_USERNAME,
        password: process.env.DEPLOY_FTP_PASSWORD,
        host: "localhost",
        port: 2121,
        localRoot: local,
        remoteRoot: remote
    };
};

var chatBotRequest = {
    apiKey: process.env.DEPLOY_DUGA_KEY,
    roomId: 16134,
    text: "New web client version uploaded to http://play.cardshifter.com/."
};

var chatBotConfig = {
    url: "http://stats.zomis.net/GithubHookSEChatService/bot/jsonPost",
    method: "POST",
    headers: {
        "Content-Type": "application/json"
    }
}

function postToChat(config, botRequest) {
    return new Promise(function(resolve, reject) {
        var json = JSON.stringify(botRequest, ["apiKey", "roomId", "text"]);
        config.headers["Content-Length"] = json.length;
        config.body = json;

        request(config)
        .then(function(body) {
            resolve(body);
        })
    });
}

function setupFiles() {
    // ftp-deploy doesn't handle uploading from multiple directories well
    return new Promise(function(resolve, reject) {
        temp.mkdir("cardshifter-deploy", function (err, tempDir) {
            if (err) {
                reject(err);
            }

            Promise.all([
                copy(path.join(__dirname, "..", "www"), tempDir),
                copy(path.join(__dirname, "..", "dist"), path.join(tempDir, "assets"))
            ])
            .then(function() {
                resolve(tempDir);
            })
            .catch(function(err) {
                reject(err);
            });
        });
    });
}

function deployFtp(config) {
    return new Promise(function(resolve, reject) {
        new FtpDeploy().deploy(config, function(err) {
            if (err) {
                reject(err);
            } else {
                resolve();
            }
        });
    });
}

setupFiles()
.then(function(dir) {
    var config = ftpConfig(dir, "/");
    console.log("Deploying to ftp://" + config.host + ":" + config.port + "...");
    return deployFtp(config);
})
.then(function() {
    console.log("FTP deployment successful.");
    if (chatBotRequest.apiKey) {
        console.log("Posting message to " + chatBotConfig.url + "...");
        return postToChat(chatBotConfig, chatBotRequest);
    }
})
.then(function(responseBody) {
    if (responseBody) {
        console.log(responseBody);
    }
})
.catch(function(err) {
    console.error("Error: " + err);
});
\$\endgroup\$
  • 1
    \$\begingroup\$ Why don't you guys use Amazon S3 for static assets? It's not free but it's cheap enough to essentially be free (something crazy like 0.9c for 10k requests) \$\endgroup\$ – Dan Sep 14 '15 at 10:14
  • 1
    \$\begingroup\$ One other thing you might look into is gulp. Particularly as it has modules for ftp, and all of the related stuff there. I'm not familiar with Duga though, so I can't help much there. \$\endgroup\$ – Seiyria Sep 14 '15 at 15:05
  • \$\begingroup\$ Server address? Try play.cardshifter.com ;) \$\endgroup\$ – Simon Forsberg Sep 14 '15 at 16:47
5
\$\begingroup\$

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();
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Are host, port and roomId really that magic? They're not everywhere: I put all configuration in easy to edit objects at the top of the file. \$\endgroup\$ – jacwah Sep 14 '15 at 9:15
  • \$\begingroup\$ Well, the roomId is pretty magic, the others are somewhat magic \$\endgroup\$ – Quill Sep 14 '15 at 9:18
  • 1
    \$\begingroup\$ Is there a reason why you would omit variable declarations? In my opinion it makes it harder to read and harder to add more variables. As you might forget to overlook the ; and ,. \$\endgroup\$ – woutr_be Sep 14 '15 at 9:51
  • 1
    \$\begingroup\$ Don't omit var. It's harder to maintain and arguably doesn't improve readability at all. This is especially true with ES6 around the corner, where you distinguish between const and let. \$\endgroup\$ – Dan Sep 14 '15 at 11:40
  • 1
    \$\begingroup\$ I'd disagree about funny comments. If they're used sparingly, like in this case, I'd say they're okay. \$\endgroup\$ – Ethan Bierlein Sep 14 '15 at 12:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.