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: remove redundant return from VecCoder.decode() #971

Merged

Conversation

danielbate
Copy link
Member

@danielbate danielbate commented May 3, 2023

The decode function is not currently supported by the VecCoder. Currently it throws an error informing the consumer of the non implementation, however a redundant return statement is still in place.

decode(data: Uint8Array, offset: number): [DecodedValueOf<TCoder>, number] {
this.throwError('unexpected Vec decode', 'not implemented');
return [undefined as unknown as DecodedValueOf<TCoder>, offset];
}

Someone reading through the source may misinterpret the implementation. This is also misreporting on code coverage.

Code coverage of decode is provided in #936

Copy link
Member

@arboleya arboleya left a comment

Choose a reason for hiding this comment

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

Nice catch.

@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2023

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
95.18% (+0.02% 🔼)
4718/4957
🟢 Branches 83.96% 832/991
🟢 Functions 92.65% 908/980
🟢 Lines
95.19% (+0.02% 🔼)
4509/4737

Test suite run success

825 tests passing in 128 suites.

Report generated by 🧪jest coverage report action from 336d4b0

@danielbate danielbate enabled auto-merge (squash) May 3, 2023 13:12
@danielbate danielbate merged commit 41da365 into master May 3, 2023
@danielbate danielbate deleted the db/refactor/remove-redundant-return-from-vec-coder-decode branch May 3, 2023 14:50
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