Skip to content
This repository was archived by the owner on Dec 2, 2024. It is now read-only.
/ party Public archive

fix: modern Node#26

Merged
vweevers merged 9 commits into
Level:masterfrom
ronag:modernize
Mar 7, 2021
Merged

fix: modern Node#26
vweevers merged 9 commits into
Level:masterfrom
ronag:modernize

Conversation

@ronag
Copy link
Copy Markdown
Contributor

@ronag ronag commented Mar 6, 2021

No description provided.

@ronag ronag force-pushed the modernize branch 6 times, most recently from 2a5eae0 to b4a4f68 Compare March 6, 2021 17:32
Copy link
Copy Markdown
Member

@vweevers vweevers left a comment

Choose a reason for hiding this comment

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

Please keep the shutdown, onlistening and onopen functions, they make the code (as well as the diff of this PR) more readable. And where there's no this to worry about, let's also keep function in favor of arrow functions.

@vweevers vweevers added the semver-major Changes that break backward compatibility label Mar 6, 2021
@ronag ronag force-pushed the modernize branch 2 times, most recently from b1fe28f to a412073 Compare March 6, 2021 23:06
@ronag ronag requested a review from vweevers March 6, 2021 23:07
@ronag
Copy link
Copy Markdown
Contributor Author

ronag commented Mar 6, 2021

Done

Comment thread index.js
Comment thread index.js
Comment thread index.js Outdated
@ronag ronag requested a review from vweevers March 7, 2021 09:22
Copy link
Copy Markdown
Member

@vweevers vweevers left a comment

Choose a reason for hiding this comment

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

Thanks, looks good! One last nit: remove the lockfile. Lockfiles are good for apps, not so much for modules. It doesn't apply when one does npm i level-party so as module authors we shouldn't lock either.

@ronag
Copy link
Copy Markdown
Contributor Author

ronag commented Mar 7, 2021

Removed

@ronag ronag requested a review from vweevers March 7, 2021 09:29
@vweevers vweevers merged commit 9615cac into Level:master Mar 7, 2021
@vweevers
Copy link
Copy Markdown
Member

vweevers commented Mar 7, 2021

5.0.0

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

Labels

semver-major Changes that break backward compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants