KristofferC commented 4 years ago

This makes test pass on Julia master. It also allows showing Records in e.g. the REPL while previously it would error with:

julia> rec = DefaultRecord("Logger.example", "info", 20, "blah")
Error showing value of type DefaultRecord:
ERROR: MethodError: no method matching length(::DefaultRecord)
Closest candidates are:
  length(::Cmd) at process.jl:639
  length(::CompositeException) at task.jl:41
  length(::Base.MethodList) at reflection.jl:869
  ...
Stacktrace:
 [1] summary(::IOContext{REPL.Terminals.TTYTerminal}, ::DefaultRecord) at ./abstractdict.jl:34

codecov[bot] commented 4 years ago

Codecov Report

Merging #165 into master will increase coverage by 1.54%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #165      +/-   ##
==========================================
+ Coverage   95.19%   96.73%   +1.54%     
==========================================
  Files          13       13              
  Lines         333      337       +4     
==========================================
+ Hits          317      326       +9     
+ Misses         16       11       -5

Impacted Files	Coverage Δ
src/records.jl	`86.66% <0.00%> (-2.62%)`	:arrow_down:
src/syslog.jl	`66.66% <0.00%> (-33.34%)`	:arrow_down:
src/exceptions.jl	`50.00% <0.00%> (-16.67%)`	:arrow_down:
src/loggers.jl	`99.09% <0.00%> (+0.90%)`	:arrow_up:
src/handlers.jl	`100.00% <0.00%> (+5.76%)`	:arrow_up:
src/formatters.jl	`98.03% <0.00%> (+5.88%)`	:arrow_up:
src/stdlib.jl	`88.88% <0.00%> (+11.11%)`	: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 ad32ea8...678883d. Read the comment docs.

rofinn commented 4 years ago

Thanks, is this a new requirement of any subtype of AbstractDict?

KristofferC commented 4 years ago

Well, the problem is that HasLength is true by default and you didn't provide a length method so you were technically breaking the interface. Right now, the dictionary constructor will try to use the length method if HasLength is true for some performance win, which it didn't use to.

rofinn commented 4 years ago

Ahh, okay, thanks.

KristofferC commented 3 years ago

Could there be a version bump with this in it?

rofinn commented 3 years ago

Sorry, done. https://github.com/JuliaRegistries/General/pull/24900

invenia / Memento.jl

implement Base.length for Record #165

Codecov Report