Skip to content

Conversation

@quinnj
Copy link
Member

@quinnj quinnj commented Apr 23, 2021

I noticed in some serialized JSON from JSON3.jl recently that some
fields were still getting serialized when I had defined
StructTypes.omitempties(::Type{MyType}) = true, which is a shorthand
we introduced when you want to omit empty values for any field in a
type. Unfortunately, we failed to update StructTypes.foreachfield to
respect this option and double unfortunately, the code still happened to
"work", i.e. not throw an error whether omitempties returned a Bool
or Tuple. The fix is pretty simple: if omitempties returns true,
then we query the fieldnames of the struct.

I noticed in some serialized JSON from JSON3.jl recently that some
fields were still getting serialized when I had defined
`StructTypes.omitempties(::Type{MyType}) = true`, which is a shorthand
we introduced when you want to omit empty values for any field in a
type. Unfortunately, we failed to update `StructTypes.foreachfield` to
respect this option and double unfortunately, the code still happened to
"work", i.e. not throw an error whether `omitempties` returned a `Bool`
or `Tuple`. The fix is pretty simple: if `omitempties` returns `true`,
then we query the `fieldnames` of the struct.
@codecov
Copy link

codecov bot commented Apr 23, 2021

Codecov Report

Merging #50 (248406e) into main (ba402e1) will increase coverage by 0.20%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   69.16%   69.36%   +0.20%     
==========================================
  Files           1        1              
  Lines         334      333       -1     
==========================================
  Hits          231      231              
+ Misses        103      102       -1     
Impacted Files Coverage Δ
src/StructTypes.jl 69.36% <100.00%> (+0.20%) ⬆️

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 ba402e1...248406e. Read the comment docs.

@quinnj quinnj merged commit b108d05 into main Apr 23, 2021
@quinnj quinnj deleted the jq/omitemp branch April 23, 2021 05:52
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.

2 participants