Skip to content

Commit

Permalink
Require aperture index rather than commands directly.
Browse files Browse the repository at this point in the history
  • Loading branch information
@timoxley
timoxley committed Apr 26, 2014
1 parent 66107e1 commit 6431c51
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions aperture.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ var config = require('./commands/config')
var resolve = require('path').resolve
var optimist = require('optimist')
var chalk = require('chalk')
var aperture = require('./')
var fs = require('fs')
var commands = {}

Expand Down Expand Up @@ -56,7 +57,7 @@ function help() {

function defineCommands() {
commands.link = function(root, config, events, done) {
require('./commands/link')(
aperture.link(

This comment has been minimized.

Sign in to view

Copy link
@hughsk

hughsk Apr 27, 2014

Member

The reason for doing the inline requires is to reduce initial startup time – unfortunately it's really hard to measure, because either Node or the OS caches the results after the first run. See also: gulpjs/gulp#282

What do you reckon?

This comment has been minimized.

Sign in to view

Copy link
@timoxley

timoxley Apr 27, 2014

Author Member

sure… but are we talking about shaving a single millisecond or maybe two? as in, the difference isn't 100s of files… it's like 5 files vs 2 files and this isn't something that's being run inside a loop is it?

This comment has been minimized.

Sign in to view

Copy link
@timoxley

timoxley Apr 27, 2014

Author Member

Oh, of course, each command has different dependencies…

This comment has been minimized.

Sign in to view

Copy link
@timoxley

timoxley Apr 27, 2014

Author Member

But yeah, how much time are we realistically saving?

This comment has been minimized.

Sign in to view

Copy link
@hughsk

hughsk Apr 27, 2014

Member

Yeah, just did a check – aperture open loads up 59 files, whereas aperture list loads up 20. Looks there's less requiring going on than I thought so think it should be fine, can switch back later if we notice it's an issue :)

This comment has been minimized.

Sign in to view

Copy link
@timoxley

timoxley Apr 27, 2014

Author Member

Ok fine, it's a minor change, I'll fix.
root
, config
, events.on('link', log)
Expand All @@ -69,7 +70,7 @@ function defineCommands() {
}

commands.purge = function(root, config, events, done) {
require('./commands/purge')(
aperture.purge(
root
, config
, events.on('queued', console.log)
Expand All @@ -81,7 +82,7 @@ function defineCommands() {
commands.install = function(root, config, events, done) {
console.log('checking package versions...')

require('./commands/install')(
aperture.install(
root
, config
, events.on('info progress', function(p) {
Expand All @@ -108,7 +109,7 @@ function defineCommands() {
'command object.'
))

require('./commands/bulk')(
aperture.bulk(
root
, config
, events
Expand Down Expand Up @@ -142,7 +143,7 @@ function defineCommands() {
process.stdout.write('% \r')
})

require('./commands/open')(
aperture.open(
root
, config
, events
Expand All @@ -155,7 +156,7 @@ function defineCommands() {
}

commands.list = function(root, config, events, done) {
require('./commands/list')(
aperture.list(
root
, config
, function(err, modules) {
Expand Down

0 comments on commit 6431c51

Please sign in to comment.