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

feat: add encoding / decoding for bytes[CompactBytesArray] #261

Merged
merged 10 commits into from
Jan 24, 2023

Conversation

CJ42
Copy link
Collaborator

@CJ42 CJ42 commented Jan 19, 2023

Co-authored-by: b00ste.lyx [email protected]

What kind of change does this PR introduce (bug fix, feature, docs update, ...)?

⭐ feature

What is the current behaviour (you can also link to an open issue here)?

There is currently no support for encoding CompactBytesArray defined in LSP2.

Related to issue #252

What is the new behaviour (if this is a feature change)?

Add basic support for bytes[CompactBytesArray]

Other information:

Copy link
Collaborator

@skimaharvey skimaharvey left a comment

Choose a reason for hiding this comment

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

LGTM. May be review the comment about empty bytes arrays

src/lib/encoder.ts Outdated Show resolved Hide resolved
src/lib/encoder.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@CallumGrindle CallumGrindle left a comment

Choose a reason for hiding this comment

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

I lack some bytes know how here but lgtm. Please update the docs too :)

Copy link
Contributor

@magalimorin18 magalimorin18 left a comment

Choose a reason for hiding this comment

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

Looks good to me too,
Very clear and easy to read code Thank you!

src/lib/encoder.test.ts Outdated Show resolved Hide resolved
src/lib/encoder.test.ts Outdated Show resolved Hide resolved
src/lib/encoder.ts Outdated Show resolved Hide resolved
src/lib/encoder.ts Outdated Show resolved Hide resolved
src/lib/encoder.ts Outdated Show resolved Hide resolved
src/lib/encoder.ts Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jan 24, 2023

Codecov Report

Merging #261 (5064b0e) into develop (874f168) will increase coverage by 1.18%.
The diff coverage is 90.16%.

@@             Coverage Diff             @@
##           develop     #261      +/-   ##
===========================================
+ Coverage    81.83%   83.02%   +1.18%     
===========================================
  Files           15       18       +3     
  Lines          914      978      +64     
  Branches       200      218      +18     
===========================================
+ Hits           748      812      +64     
- Misses          90       91       +1     
+ Partials        76       75       -1     
Impacted Files Coverage Δ
src/constants/constants.ts 100.00% <ø> (ø)
src/lib/getDataFromExternalSources.ts 64.51% <ø> (ø)
src/lib/isValidSignature.ts 75.00% <ø> (+8.33%) ⬆️
src/lib/utils.ts 83.89% <ø> (ø)
src/types/Method.ts 100.00% <ø> (ø)
src/index.ts 77.00% <85.71%> (-4.54%) ⬇️
src/lib/provider-wrapper-utils.ts 89.47% <85.71%> (+0.58%) ⬆️
src/lib/detector.ts 87.50% <87.50%> (ø)
src/lib/decodeMappingKey.ts 89.74% <89.74%> (ø)
src/lib/getData.ts 89.79% <89.79%> (ø)
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Hugoo
Copy link
Contributor

Hugoo commented Jan 24, 2023

This is weird, like why would the test coverage change on these files? 🤔
image

@Hugoo Hugoo merged commit 8d3e4e9 into develop Jan 24, 2023
@Hugoo Hugoo deleted the feat/compact-bytes-array branch January 24, 2023 11:09
@Hugoo Hugoo mentioned this pull request Aug 3, 2023
This was referenced May 15, 2024
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.

7 participants