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

popm/wasm: improve js.Value creation, add JSMarshaler interface #160

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

joshuasing
Copy link
Contributor

Summary
Improve js.Value creation in the WebAssembly PoP Miner, add JSMarshaler interface.

Changes

  • Refactor logic of jsValueOf and jsValueSafe
  • Improve error logging when a value cannot be converted
  • Add JSMarshaler interface which can be used to implement custom logic for converting a type into a js.Value

@joshuasing joshuasing added type: refactor This refactors existing functionality size: M This change is medium (+/- <200) area: popm/wasm This is a change to the WASM popm (PoP Miner) labels Jun 27, 2024
@joshuasing joshuasing requested a review from a team as a code owner June 27, 2024 14:45
Copy link
Contributor

@ClaytonNorthey92 ClaytonNorthey92 left a comment

Choose a reason for hiding this comment

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

hey @joshuasing my feedback here is optional; I would just suggest adding some unit tests especially for jsValueSafe. There seem to be a lot of conditions there, it seems like a (very) good candidate for table driven tests of what to expect. and I feel that we should protect ourselves here.

Copy link
Contributor

@marcopeereboom marcopeereboom left a comment

Choose a reason for hiding this comment

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

Cute.

@joshuasing joshuasing merged commit 526787a into main Jun 27, 2024
4 checks passed
@joshuasing joshuasing deleted the joshua/wasm-popm-improve-values branch June 27, 2024 17:12
web3cryptoguy pushed a commit to web3cryptoguy/heminetwork that referenced this pull request Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: popm/wasm This is a change to the WASM popm (PoP Miner) size: M This change is medium (+/- <200) type: refactor This refactors existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants