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

Alternative Field Encoding for Types #33

Merged
merged 5 commits into from
Sep 10, 2024
Merged

Conversation

Doreen-Schwartz
Copy link
Contributor

@Doreen-Schwartz Doreen-Schwartz commented Aug 15, 2024

Changes

  • Added the option to pass an alternative encoder (informationWriter) to each Field for nistEncode.
  • Added the option to pass an alternative decoder (informationDecoder) to each Field for nistDecode.

@ivosh
Copy link
Owner

ivosh commented Aug 15, 2024

@Doreen-Schwartz The interface changes look good! Please proceed with the rest of the implementation.

@Doreen-Schwartz Doreen-Schwartz changed the title WIP: Alternative Field Encoding for Types Alternative Field Encoding for Types Sep 9, 2024
@Doreen-Schwartz Doreen-Schwartz marked this pull request as ready for review September 9, 2024 14:53
@Doreen-Schwartz
Copy link
Contributor Author

I've added in unit tests + the requested changes. I think it's also worth adding this feature somewhere in the README, but I wasn't sure where would be most appropriate.

src/nistEncode.ts Outdated Show resolved Hide resolved
@ivosh
Copy link
Owner

ivosh commented Sep 10, 2024

@Doreen-Schwartz The PR looks really good! Thank you for the unit tests.
Please can you address the review comment. Then I will merge the PR, add something in README and publish a new version.

@Doreen-Schwartz
Copy link
Contributor Author

@ivosh Thank you for the review. I've added a commit with a correction based on your comment

@ivosh
Copy link
Owner

ivosh commented Sep 10, 2024

@Doreen-Schwartz Please could you fix the formatting problem reported by eslint:
https://github.com/ivosh/node-nist/actions/runs/10788163581/job/29929082522?pr=33
The CI pipeline fails because of this check.
Thanks!

@Doreen-Schwartz
Copy link
Contributor Author

@ivosh Fixed!

Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.59%. Comparing base (038b287) to head (49611d8).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #33      +/-   ##
==========================================
+ Coverage   96.52%   96.59%   +0.06%     
==========================================
  Files          10       10              
  Lines         778      793      +15     
  Branches      230      235       +5     
==========================================
+ Hits          751      766      +15     
- Misses         26       27       +1     
+ Partials        1        0       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ivosh ivosh merged commit 7793620 into ivosh:master Sep 10, 2024
6 checks passed
@Doreen-Schwartz
Copy link
Contributor Author

@ivosh Thanks for merging the PR! When can I expect this feature to be released onto npm so it can be used in my project?

@ivosh
Copy link
Owner

ivosh commented Sep 11, 2024

I am currently working on upgrading dependencies and updating README.

@ivosh ivosh added the enhancement New feature or request label Sep 11, 2024
@ivosh ivosh linked an issue Sep 11, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WIN-1255 Encoding for Type 2 Fields
2 participants