Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Return future when running a command #420

Merged
merged 1 commit into from
Nov 13, 2017
Merged

Conversation

ludat
Copy link
Contributor

@ludat ludat commented Nov 9, 2017

This should fix #415

@ludat
Copy link
Contributor Author

ludat commented Nov 9, 2017

It looks like CI is failing because of linting errors on a file I didn't touch

@ludat ludat changed the title [neutrino-middleware-eslint] Turn Fututre into Promise to be evaluated [neutrino-middleware-eslint] Turn Future into Promise to be evaluated Nov 9, 2017
Copy link
Member

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason converting this Future to a Promise fixes this bug is because it forces the Future to eagerly evaluate. The Neutrino API's run method is designed to allow the returning of Futures from the result of running a command, so there must be something else going on here.

Walking through the code, it appears that invoking the command function doesn't actually return anything. So instead of making the change you have here, could you modify packages/neutrino/bin/base.js line 20, to return the value of running the command function? That should indeed fix this problem for all commands.

@ludat
Copy link
Contributor Author

ludat commented Nov 9, 2017

@eliperelman yup, you were exactly right. I just updated the PR

@ludat ludat changed the title [neutrino-middleware-eslint] Turn Future into Promise to be evaluated Return future when running a command Nov 9, 2017
Copy link
Member

@eliperelman eliperelman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

@eliperelman eliperelman merged commit 566f5c6 into neutrinojs:master Nov 13, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

neutrino lint has broken after upgrade to neutrino@7.3.0
2 participants