Skip to content
This repository has been archived by the owner on Nov 21, 2018. It is now read-only.

Refactors/simplify the “Templates” task in a more gulp-like way #257

Merged
merged 1 commit into from
Mar 4, 2015

Conversation

snostorm
Copy link
Contributor

@snostorm snostorm commented Mar 3, 2015

Open questions:

  • Discussion on whether or not we want to still continue to include build information as per Added build information into the template build process #168 (it might be best to make this a production-environment flag for when we generate automatic builds.) If so, it can be added back prior to merging.
  • What dependencies in the package.json are no longer needed

To do (non-blockers):

  • Proof-of-concept metadata tracking around using alternative templates on certain articles
  • Tracking a manifest of what articles are to be created (for i18n cross-linking.)
  • Proof-of-concept page that is not markdown based. Example: the homepage shouldn't really be a markdown file, but I may make a new page (like downloads/ release history/ etc.) as a test.
  • Live reload integration

Notes:

  • Adds a lot of commenting, the refactor may be easier to review as the final file versus reading a diff.
  • Snuck in a fix for Localize comment in HTML #225
  • This can be merged as-is. The output public/**/*.html is the same, except for the removal of the build information. (The non-blockers above can land later.)

Edit: I originally reported the refactor was (suspiciously) slower now than the original. But, in fact, because Gulp is reporting the original version of this task as completed near-instantly and not factoring in work which was is processing, this benchmark is incorrect.

On my modern MacBook Pro I'm getting about 1700ms less runtime with the refactor when manually benchmarking gulp templates with a process.on('exit', /* ... */) hook shared between the two branches:

console.time("gulp");

// Require all tasks in gulp/tasks, including subfolders
requireDir('./gulp/tasks', { recurse: true });

process.on('exit', function() { console.timeEnd("gulp"); });

To be fair, this refactor also does less work (no git log parsing, no checksum lookup/checks.)

@therebelrobot
Copy link
Contributor

I'll take a look at these this week as well :) +1 to merge anytime though.

var exec = require('child_process').exec
var utils = require('../util/template-utils.js');
/*
* 1. because this is a gulp task. duh.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@snostorm
Copy link
Contributor Author

snostorm commented Mar 4, 2015

This was more-or-less approved via the sister PR. Since I'm still waiting on some i18n feedback I'm going to land this portion first. Regarding the nice-to-have portions above I'll transition those to a new Issue.

snostorm added a commit that referenced this pull request Mar 4, 2015
Refactors/simplify the “Templates” task in a more gulp-like way
@snostorm snostorm merged commit e553c29 into master Mar 4, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants