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

refactor: use ES2015 #31

Merged
merged 1 commit into from
Jul 19, 2019
Merged

refactor: use ES2015 #31

merged 1 commit into from
Jul 19, 2019

Conversation

demurgos
Copy link
Contributor

@demurgos demurgos commented Oct 9, 2018

Why

Following #24, the library requires Node 6. This allows to use ES2015 features to make the code more expressive and easier to maintain.

What

Replace var declarations by const or let. Use arrow functions. Replace arguments slicing with rest arguments. Use Map to store the listeners.

Copy link
Member

@coreyfarrell coreyfarrell left a comment

Choose a reason for hiding this comment

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

LGTM @isaacs @bcoe

@isaacs
Copy link
Member

isaacs commented May 23, 2019

+1, but this has conflicts with master, please rebase.

@coreyfarrell
Copy link
Member

@demurgos do you have a chance to work on this?

@demurgos
Copy link
Contributor Author

Thanks for the reminder @coreyfarrell
I'll rebase the commit and fix the conflicts.

@demurgos
Copy link
Contributor Author

I rebased the PR and fixed the conflicts. The only change was in the proxySignals function (extra parameter).

@demurgos
Copy link
Contributor Author

demurgos commented Jul 18, 2019

I am looking into the errors: they seem to also be present on master.

@demurgos
Copy link
Contributor Author

demurgos commented Jul 18, 2019

CI is failing on Node 6 because tap dropped support for Node 6. It passes on Node 8. I don't know why it fails on Node 10. It passes locally using Node 10: is the test flaky?

# Why

Following tapjs#24, the library requires Node 6. This allows to use ES2015 features to make the code more expressive and easier to maintain.

# What

Replace `var` declarations by `const` or `let`. Use arrow functions. Replace `arguments` slicing with rest arguments. Use `Map` to store the listeners.
@coreyfarrell coreyfarrell merged commit e29c794 into tapjs:master Jul 19, 2019
@demurgos demurgos deleted the es2015 branch July 19, 2019 16:19
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.

3 participants