Add output functionality #24
Conversation
src/lib/invoke.js
Outdated
| 'X-Build-Meta-OS': `${os.type().toLowerCase()}@${os.release()}`, | ||
| }; | ||
|
|
||
| if (data['x-build-outputs'] !== undefined) { |
There was a problem hiding this comment.
Instead of passing it through data['x-build-outputs'], why not just pass it as a param into invoke?
There was a problem hiding this comment.
Yeah I thought about that..
But the last parameter is optional..
I could make it so invoke takes another param. But then I'd have to change this line to be:
return invoke(scope.key, scope.service, action, data, undefined, outputs).then((response) => {
Long story short:
- I didn't want to add a parameter and then have to pass in
undefinedthere. - I didn't want to change the order of the parameters coming in.
But if we really don't like this.. I'm happy to change it so that we pass in this object so that there isn't an order dependency anymore!
{
key: '',
service: '',
action: '' ,
data: '',
isCLI : ''
}
There was a problem hiding this comment.
What about doing this:
invoke(key, service, action, data, opts)And making ops have isCLI, outputs and whatever else we decide to add in the future?
I'm also not against just making it an object.
There was a problem hiding this comment.
I quite like just making it an object.
src/api.js
Outdated
| const linksPath = path.join(utils.sharedDirectoryPath(), 'links.json'); | ||
|
|
||
| module.exports.run = (action, d, cb) => { | ||
| module.exports.run = (action, d, outputs, cb) => { |
There was a problem hiding this comment.
Doesnt this mean that if you want to use run with a cb that you have to pass in something for outputs?
const api = require('api').config('domh_ccd5142d2107c48ce32f75');
api('build-test')
.run('helloWorld', { name: 'Name' },
// this has to be here now?
{},
function cb () {})Can you make it optional?
There was a problem hiding this comment.
@domharrington fixed!
| if (data['x-build-outputs'] !== undefined) { | ||
| const parsedOutput = querystring.stringify(data['x-build-outputs']); | ||
| delete data['x-build-outputs']; | ||
| headers['X-Build-Output'] = parsedOutput; |
There was a problem hiding this comment.
Can you add a test here to make sure the header gets set? https://github.com/readmeio/api/blob/master/test/commands/run.test.js
There was a problem hiding this comment.
@domharrington is this file for the .run function?
3 participants
No description provided.