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

Conversion from nodeback to task #116

Merged
merged 1 commit into from Apr 26, 2017

Conversation

Projects
None yet
2 participants
@rpearce
Contributor

rpearce commented Apr 25, 2017

This is my first foray into this project, so please be gentle!

Couple of housekeeping things:

  • I'm not sure what to put in the type magic doc field for this. Could you help me understand that better?
  • I did my best to conform to your code style on this one. Any changes required?
  • is this happy-path test enough? what else would you like to see?
describe('#fromNodeback', () => {
  it('Returns a task when given a nodeback-style function', () => {
    let resultStr = 'test';
    const fn = (str, cb) => cb(str + '-processed');
    const convertedFn = Task.fromNodeback(fn);
    const appliedFn = convertedFn(resultStr, (str) => {
      resultStr = str;
    });

    appliedFn.run();

    $ASSERT(resultStr === 'test-processed');
  });
});

This closes #96

EDIT: I've noticed this deleted a lot of files... looking at it. I only ran the make commands as specified in the docs
EDIT2: it also looks like I need to update my commit message stuff. Argh. Any guidance is appreciated.

To Do

  • write documentation
  • update commit message
  • un-delete docs files(?)
  • add spec for resolving successfully with a value
  • add spec for error case

@rpearce rpearce changed the title from Conversion from nodeback to task to WIP :: Conversion from nodeback to task Apr 25, 2017

@robotlolita

This comment has been minimized.

Show comment
Hide comment
@robotlolita

robotlolita Apr 25, 2017

Member

screenshot from 2017-04-25 20-10-17

Thanks for the time in putting together this PR :)

Ah, yeah, the Make commands will delete the generated documentation :x You can regenerate it with make documentation. I have to move these to the gh-pages branch :x

As for the git commit message, you can rebase after you're done and reword/squash your commits as needed. Don't worry too much about it though :)


So, on to the other things.

The type documentation field is pretty much a description of how the inputs and outputs like following this notation. In this case, the types would look like this:

forall s, e, r:
((Any..., (e, s) => Void) => Void)
=> (Any...)
=> Task e s r

Where s is the success value, e is the error, and r is a resource type.

The style is a-okay :) I've added a couple of notes on the patch.

Member

robotlolita commented Apr 25, 2017

screenshot from 2017-04-25 20-10-17

Thanks for the time in putting together this PR :)

Ah, yeah, the Make commands will delete the generated documentation :x You can regenerate it with make documentation. I have to move these to the gh-pages branch :x

As for the git commit message, you can rebase after you're done and reword/squash your commits as needed. Don't worry too much about it though :)


So, on to the other things.

The type documentation field is pretty much a description of how the inputs and outputs like following this notation. In this case, the types would look like this:

forall s, e, r:
((Any..., (e, s) => Void) => Void)
=> (Any...)
=> Task e s r

Where s is the success value, e is the error, and r is a resource type.

The style is a-okay :) I've added a couple of notes on the patch.

@rpearce

This comment has been minimized.

Show comment
Hide comment
@rpearce

rpearce Apr 25, 2017

Contributor

@robotlolita thank you! Where can I see these notes you referenced?

I'll be getting to this in a few hours. Thanks again

Contributor

rpearce commented Apr 25, 2017

@robotlolita thank you! Where can I see these notes you referenced?

I'll be getting to this in a few hours. Thanks again

@robotlolita

This comment has been minimized.

Show comment
Hide comment
@robotlolita

robotlolita Apr 26, 2017

Member

@rpearce oh right, it doesn't show up if I don't submit them. Sorry about that :')

Member

robotlolita commented Apr 26, 2017

@rpearce oh right, it doesn't show up if I don't submit them. Sorry about that :')

(new task test) Implements Task.fromNodeback
 will allow you to pass node-style callback-accepting functions,
also known as "nodebacks", and convert them to Tasks.

Closes #96

@rpearce rpearce changed the title from WIP :: Conversion from nodeback to task to Conversion from nodeback to task Apr 26, 2017

@rpearce

This comment has been minimized.

Show comment
Hide comment
@rpearce

rpearce Apr 26, 2017

Contributor

@robotlolita mind taking another look at this? The commit message, tests, and all the rest should meet the requirements now!

Contributor

rpearce commented Apr 26, 2017

@robotlolita mind taking another look at this? The commit message, tests, and all the rest should meet the requirements now!

@robotlolita robotlolita merged commit e2f9fad into origamitower:master Apr 26, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@robotlolita

This comment has been minimized.

Show comment
Hide comment
@robotlolita

robotlolita Apr 26, 2017

Member

Thanksies :)

Member

robotlolita commented Apr 26, 2017

Thanksies :)

@rpearce rpearce deleted the rpearce:from-nodeback branch Apr 26, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment