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

Fix lint #182

Merged
merged 3 commits into from Jul 30, 2019
Merged

Fix lint #182

merged 3 commits into from Jul 30, 2019

Conversation

schmidsi
Copy link
Contributor

@schmidsi schmidsi commented Jul 19, 2019

Make standard run. It was mostly just running standard --fix. But changes in main/signers/hot/RingSigner/worker.js and in main/signers/hot/SeedSigner/worker.js had to be made by hand.

As far as I understand the logic, this is just initialisation code to run the code in the constructor. So theoretically, we could omit the variable declaration and just run new SeedSignerWorker(). But then, Eslint will throw the Do not use 'new' for side effects.. So I just ignore this line with // eslint-disable-line, but it would probably be cleaner to refactor the code so that the constructor does not have side effects: Move process.on('message', (message) => this.handleMessage(message)) outside of the constructor and run it directly on the files root level: process.on('message', (message) => seedSignerWorker.handleMessage(message))

Thoughts? @monkybrain

@schmidsi schmidsi mentioned this pull request Jul 19, 2019
2 tasks
@floating
Copy link
Owner

LGTM, will merge after 0.2 audit is complete (hopefully later today)

@floating floating merged commit daf7003 into floating:0.2 Jul 30, 2019
@schmidsi schmidsi deleted the fix-lint branch August 28, 2019 08:35
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.

None yet

2 participants