search

KristofferC / PGFPlotsX.jl

Plots in Julia using the PGFPlots LaTeX package
Other
301 stars 40 forks source link

Make push! and append! return the original object #215

Closed fredrikekre closed 4 years ago

codecov-io commented 4 years ago

Codecov Report

Merging #215 into master will decrease coverage by 0.24%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #215      +/-   ##
==========================================
- Coverage   86.63%   86.38%   -0.25%     
==========================================
  Files           9        9              
  Lines         591      617      +26     
==========================================
+ Hits          512      533      +21     
- Misses         79       84       +5
Impacted Files Coverage Δ
src/PGFPlotsX.jl 90.47% <ø> (ø) :arrow_up:
src/tikzpicture.jl 100% <100%> (ø) :arrow_up:
src/options.jl 90.9% <100%> (+0.9%) :arrow_up:
src/axislike.jl 100% <100%> (+2.43%) :arrow_up:
src/tikzdocument.jl 69.62% <100%> (+0.08%) :arrow_up:
src/axiselements.jl 93.33% <100%> (-2.02%) :arrow_down:

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 e87e240...baef596. Read the comment docs.

tpapp commented 4 years ago

This introduces a lot code repetition. If we are using MacroTools anyway, why not just fix @forward?

Or copy a fixed version of it into this package. If I understand correctly, all we need is remove the nothing.

fredrikekre commented 4 years ago

But you can't know in general what should be returned, e.g. push! should return the original collection whereas getindex should not, so you would have to implement two versions of the macro.

If I understand correctly, all we need is remove the nothing.

No, that's just the value returned from the macro.

tpapp commented 4 years ago

Thanks for the explanation, I missed that part.