New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(update): Add update command #71

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@PWKad
Contributor

PWKad commented May 23, 2015

This adds the ability to update official aurelia dependencies in one command

feat(update): Add update command
This adds the ability to update official aurelia dependencies in one command
@PWKad

This comment has been minimized.

Contributor

PWKad commented May 23, 2015

@joelcoxokc @zewa666 @ahmedshuhel The only issue remaining that I see is jspm clean is run too early once the first promise is resolved. Gladly accept any ideas for fixing that otherwise we can open an issue to add that as an enhancement to spawn-promise which should resolve that issue.

@@ -0,0 +1,14 @@
import * as logger from '../lib/logger';

This comment has been minimized.

@zewa666

zewa666 May 23, 2015

Member

So according to our last discussion should we do this one via require?

repoList.forEach(repo => {
updateCommands.push({ command: 'jspm', args: ['install', repo]})
});
SpawnPromise(updateCommands)

This comment has been minimized.

@zewa666

zewa666 May 23, 2015

Member

Guess this is what you meant with the synchronous multi execution. Guess we should, as mentioned in the gitter channel, use something like Bluebirds Promise.join feature

function moveNPMPackages(done){
fs.rename('node_modules', 'node_modules_backup', done);
}

This comment has been minimized.

@zewa666

zewa666 May 23, 2015

Member

If you'd like to rewrite those to promises you can use bluebirds promisifyAll feature. https://github.com/petkaantonov/bluebird/blob/master/API.md#promisepromisifyallobject-target--object-options---object

else
cfg[c] = v;
}
},

This comment has been minimized.

@zewa666

zewa666 May 23, 2015

Member

Phew that one's really hard to read. Now this is only my opinion so feel free to ignore if you don't like it but I'd add braces even on single step if's as it makes the code more readable. Next the variable naming convention makes understanding this section quite hard, so longer more descriptive names could help.

This comment has been minimized.

@PWKad

PWKad May 23, 2015

Contributor

Yeah most of this is borrowed from jspm config loader for now until I can find a way to take advantage of that loader through their api. I'll go through and figure out what it is doing and clean it up a bit.

// logger.log();
// logger.log(' $ aurelia plugin -a i18n');
// logger.log(' $ aurelia p -r i18n -c');
// logger.log();

This comment has been minimized.

@zewa666

zewa666 May 23, 2015

Member

Are those comments meant as an example output of the --help method above?

This comment has been minimized.

@PWKad

PWKad May 23, 2015

Contributor

Good question, I had that there uncommented before but I noticed that all of the examples were commented above in the same file. Actually while reading it those are from the plugin anyway so need to remove.

This comment has been minimized.

@joelcoxokc

joelcoxokc May 24, 2015

Contributor

I have actually not been adding braces around simple conditions... I think braces are only necessary if the condition block is large.

for example I wouldn't use braces here

if (1 === num)
    return 'yes';

This comment has been minimized.

@joelcoxokc

joelcoxokc May 24, 2015

Contributor

I like this PR. I would like to merge.

Or would you rather wait for my PR with the updated spawnPromise?

@joelcoxokc joelcoxokc assigned joelcoxokc and PWKad and unassigned joelcoxokc May 24, 2015

@joelcoxokc

This comment has been minimized.

Contributor

joelcoxokc commented May 26, 2015

@PWKad Would you like me to convert this PR to our new API?

@PWKad PWKad closed this May 28, 2015

@JeroenVinke JeroenVinke deleted the feat/add-update-command branch Feb 2, 2018

ryan-smith-virteom added a commit to ryan-smith-virteom/cli that referenced this pull request Mar 6, 2018

fix(cli build): reassign global.define.amd after assigning karma over…
…ride function

aurelia/testing ComponentTester wasn't working. Bootstrapper failed to return with an error that there was no loader defined. Tracked the error down to a dependency reassigning window.require because define.amd was undefined.

aurelia/testing issue aurelia#71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment