Skip to content
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

listSync #42

Closed
wants to merge 1 commit into from
Closed

listSync #42

wants to merge 1 commit into from

Conversation

jasonkhanlar
Copy link
Contributor

I am not able to use await cowsay.list(function(nothing, cowfiles) { return cowfiles; }) in my code, but with this change, await cowsay.listSync() returns the data.

… return cowfiles; })` in my code, but with this change, `await cowsay.listSync()` returns the data.
@piuccio
Copy link
Owner

piuccio commented Oct 26, 2018

What version of node do you use? I'd like to fix cowsay.list to return a Promise, it seems weird that you have to await a Sync() function.

Btw, because list takes a callback, you can also call

const util = require('util');
const list = util.promisify(cows.list);

await list();

@jasonkhanlar
Copy link
Contributor Author

Oh, I was wondering about that (the list() function and listSync() function was a bit confusing to me).

I was going to rewrite the list() function to return a promise, but then I noticed and tried the listSync() function, which worked for me.

Since you mention that, I'll go ahead and submit a pull request to make list() return a promise. I am using Node v11.0.0 (previously v10.12.0) in an Archlinux environment.

@jasonkhanlar
Copy link
Contributor Author

See #44

@piuccio
Copy link
Owner

piuccio commented Nov 2, 2018

We don't need listSync now that list returns a promise, you can wait on that. I'll release a new version once the new cows are merged

@piuccio piuccio closed this Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants