tanmaykm / Thrift.jl

Thrift for Julia
Other
27 stars 14 forks source link

Header format support #70

Closed tk3369 closed 2 years ago

tk3369 commented 2 years ago

This PR adds support for the "Header" format.

P.S. I haven't cleaned up my commits. It's probably better to just squash everything when this PR is merged.

codecov-commenter commented 2 years ago

Codecov Report

Merging #70 (3e236cb) into master (1f2f5ab) will increase coverage by 6.11%. The diff coverage is 83.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #70      +/-   ##
==========================================
+ Coverage   58.30%   64.42%   +6.11%     
==========================================
  Files           8        9       +1     
  Lines         969     1276     +307     
==========================================
+ Hits          565      822     +257     
- Misses        404      454      +50     
Impacted Files Coverage Δ
src/Thrift.jl 100.00% <ø> (ø)
src/server.jl 62.85% <0.00%> (-1.85%) :arrow_down:
src/protocols.jl 62.75% <70.73%> (+1.08%) :arrow_up:
src/utils.jl 73.68% <73.68%> (ø)
src/transports.jl 71.18% <86.95%> (+36.00%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1f2f5ab...3e236cb. Read the comment docs.

tk3369 commented 2 years ago

I wasn't sure why CI failed on Julia 1.3, and I wasn't able to reproduce the issue on my local desktop with Julia 1.3. Checking the code, the InfoIDEnum value should be constructed only when the value is non-zero but the test failure clearly said that it's zero. So My guess is that the if-condition may be too strict. The latest commit made it use == rather ===.

tanmaykm commented 2 years ago

Thanks!