add steps api functions #293
Conversation
Signed-off-by: tech4GT <[email protected]>
ghost
assigned 
src/util/getStep.js
Outdated
| }, | ||
|
|
||
| getHeight : function(callback){ | ||
| getPixels(this.getStep(-1).output.src,function(err,pixels){ |
There was a problem hiding this comment.
would using getPixels require parsing the whole image? I wonder if that'd be an expensive thing to do and if there are any reasonable alternatives. What do you think?
There was a problem hiding this comment.
Hmm...yeah now that you have mentioned this, yes I'll change this!! Let's get the Module requirements PR merged in and rebase this before making any more changes, then we can bump a major version!!
|
Linking to #242 Very cool! |
Signed-off-by: tech4GT <[email protected]>
Signed-off-by: tech4GT <[email protected]>
|
@jywarren merged with master!! |
|
This looks great. Can we add some tests to demonstrate usage of these? Thank you, great work! |
|
@jywarren Working on this now!! |
|
@jywarren Actually most of these are functions which are scoped inside the module... So I would either have to write a test module which uses all of these or we can add tests as we develop modules which use these, which way should we go? |
|
lets do a test module! i think thatll be worth it :-)
…On Mon, Jun 18, 2018 at 2:22 PM Varun Gupta ***@***.***> wrote:
@jywarren <https://github.com/jywarren> Actually most of these are
functions which are scoped inside the module... So I would either have to
write a test module which uses all of these or we can add tests as we
develop modules which use these, which way should we go?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#293 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ9r8FflxvakUd8CgjYLLszmfDvg8ks5t9-_dgaJpZM4Uk5zH>
.
|
|
@jywarren Working on this now!! |
|
@jywarren I got an idea while working on this!! Should we implement the importModule functionality first? I mean like we discussed in the dynamic modules idea, that way we won't have to bundle this test module with the source code, we can just register it inside the tests file and run the sequencer with it(we will keep this module inside the test folder as well) |
|
This makes a lot of sense to me since for the implementation of met-modules I have in mind this will be a welcome functionality!! Please see the meta-modules I made a flow over there... |
|
And sorry we are postponing meta-modules again and again but I think once we are done with dynamic modules core(not completely, like we can include installing from npm part later) it would help with meta-modules since meta-modules are themselves dynamic |
|
Oh, I like that. Good idea!
On Jun 19, 2018 10:05 AM, "Varun Gupta" <[email protected]> wrote:
@jywarren <https://github.com/jywarren> I got an idea while working on
this!! Should we implement the importModule functionality first? I mean
like we discussed in the dynamic modules idea, that way we won't have to
bundle this test module with the source code, we can just register it
inside the tests file and run the sequencer with it(we will keep this
module inside the test folder as well)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#293 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ5ecY2Ysh6sMQTHvPQYl5iAmKfmnks5t-QUGgaJpZM4Uk5zH>
.
|
|
@jywarren So Should we merge this in right now or rebase this over dynamic modules pr? |
|
Let's delay this until we can include the dynamic test module. Is that ok? |
|
@jywarren Sure!! So now I am doing this in a way that just by adding a few lines to the existing module file it can be made dynamically loadable!! |
|
Awesome
…On Tue, Jun 19, 2018, 11:46 AM Varun Gupta ***@***.***> wrote:
@jywarren <https://github.com/jywarren> Sure!! So now I am doing this in
a way that just by adding a few lines to the existing module file it can be
made dynamically loadable!!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#293 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ3PnaMYVKkmw660B68xiGsrytJWvks5t-Ry3gaJpZM4Uk5zH>
.
|
|
Rebasing and adding tests!! |
Signed-off-by: tech4GT <[email protected]>
Signed-off-by: tech4GT <[email protected]>
Signed-off-by: tech4GT <[email protected]>
|
@jywarren This is ready too!!🎉 |
|
@jywarren I'll try to put in the meta-modules pr as soon as possible now!! |
test/modules/testModule/Module.js
Outdated
| this.getInput(1); | ||
| this.getOutput(1); | ||
| this.getOptions(); | ||
| this.getFormat(); |
There was a problem hiding this comment.
Can we assert anything with these?
There was a problem hiding this comment.
@jywarren I thought we just wanted to check if they throw an error or not, actually som of these like height I can hardcode at best, will that be ok?
Signed-off-by: tech4GT <[email protected]>
|
@jywarren Done!! |
|
@jywarren Please merge this in. Thanks |
|
🎉 |
|
Awesome!!! |
@jywarren Added all functions🎉
Can you please elaborate on listeners.
Signed-off-by: tech4GT [email protected]