Skip to content
This repository was archived by the owner on Apr 11, 2024. It is now read-only.

#25 - Review and refactor src/crypto/ecdsa.js.#56

Merged
ClemensLey merged 3 commits intobitcoin-computer:masterfrom
paranojik:25-refactor-crypto-ecdsa
Sep 14, 2018
Merged

#25 - Review and refactor src/crypto/ecdsa.js.#56
ClemensLey merged 3 commits intobitcoin-computer:masterfrom
paranojik:25-refactor-crypto-ecdsa

Conversation

@paranojik
Copy link
Contributor

No description provided.

@paranojik paranojik changed the title Closes #25 - Review and refactor src/crypto/ecdsa.js. #25 - Review and refactor src/crypto/ecdsa.js. Sep 13, 2018
Copy link
Contributor

@slashrsm slashrsm left a comment

Choose a reason for hiding this comment

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

Made sure that:

  • The tests are still passing
  • Linter does not report any errors

Also read through the diff and all changes made sense. Ready to be merged as far as I am concerned. Thanks!

return this;
}
} catch (e) {
console.error(e); // eslint-disable-line no-console
Copy link
Contributor

Choose a reason for hiding this comment

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

We could probably use some better error handling approach? Not in scope for this issue in my opinion. Follow-up?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed. As discussed, maybe we should add a pull request for removing console logs and fix them all together.

Copy link
Contributor

Choose a reason for hiding this comment

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

Follow-up #58

@ClemensLey ClemensLey merged commit 555b868 into bitcoin-computer:master Sep 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants