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 callbacks from metadataParse #1470

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mShan0
Copy link
Contributor

@mShan0 mShan0 commented Jul 15, 2022

Would it be a good idea to create a new stream parser-style class to handle these parsers?

Planning to move the parser position reset upon retry into metadataParse.

@mShan0 mShan0 force-pushed the mark-metadata-parse-refactor branch from 8ac049f to 0ae76f5 Compare July 15, 2022 20:29
@mShan0 mShan0 marked this pull request as ready for review July 25, 2022 17:36
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #1470 (92f30ed) into master (5015634) will decrease coverage by 0.00%.
The diff coverage is 84.84%.

@@            Coverage Diff             @@
##           master    #1470      +/-   ##
==========================================
- Coverage   80.50%   80.50%   -0.01%     
==========================================
  Files          91       91              
  Lines        4669     4708      +39     
  Branches      856      860       +4     
==========================================
+ Hits         3759     3790      +31     
- Misses        634      641       +7     
- Partials      276      277       +1     
Impacted Files Coverage Δ
src/value-parser.ts 81.94% <50.00%> (ø)
src/token/colmetadata-token-parser.ts 70.83% <60.00%> (-5.36%) ⬇️
src/token/returnvalue-token-parser.ts 73.33% <70.00%> (-15.56%) ⬇️
src/metadata-parser.ts 92.20% <92.00%> (+0.20%) ⬆️
src/connection.ts 65.24% <0.00%> (+0.09%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@mShan0 mShan0 marked this pull request as draft August 10, 2022 18:03
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.

1 participant