rafaqz / Interfaces.jl

Macros to define and implement interfaces, to ensure they are checked and correct.
MIT License
72 stars 4 forks source link

Literate-based documentation #17

Closed gdalle closed 9 months ago

gdalle commented 9 months ago

Main goals:

Other documentation changes:

Other test changes:

rafaqz commented 9 months ago

+1 for adding Aqua.

This is great bit I dont use literate like this, so cant give much of a review. I'll merge anything that passes tests ;)

gdalle commented 9 months ago

+1 for adding Aqua.

The ambiguity test fails, I left it in case it is something you can fix.

2 ambiguities found
Ambiguity #1
kwcall(::NamedTuple, ::typeof(Interfaces.test), T::Type{<:Interfaces.Interface{Keys}}, O::Type, test_objects) where Keys @ Interfaces ~/Work/GitHub/Julia/Interfaces.jl/src/test.jl:20
kwcall(::NamedTuple, ::typeof(Interfaces.test), T::Type{<:Interfaces.Interface}, O::Type, objs::Interfaces.TestObjectWrapper) @ Interfaces ~/Work/GitHub/Julia/Interfaces.jl/src/test.jl:29

Possible fix, define
  kwcall(::NamedTuple, ::typeof(Interfaces.test), ::Type{<:Interfaces.Interface{Keys}}, ::Type, ::Interfaces.TestObjectWrapper) where Keys

Ambiguity #2
test(T::Type{<:Interfaces.Interface{Keys}}, O::Type, test_objects; kw...) where Keys @ Interfaces ~/Work/GitHub/Julia/Interfaces.jl/src/test.jl:20
test(T::Type{<:Interfaces.Interface}, O::Type, objs::Interfaces.TestObjectWrapper; show, keys) @ Interfaces ~/Work/GitHub/Julia/Interfaces.jl/src/test.jl:29

Possible fix, define
  test(::Type{<:Interfaces.Interface{Keys}}, ::Type, ::Interfaces.TestObjectWrapper) where Keys

Method ambiguity: Test Failed at /Users/guillaume/.julia/packages/Aqua/dInrs/src/ambiguities.jl:99
  Expression: num_ambiguities == 0
   Evaluated: 2 == 0

This is great but I dont use literate like this

How do you use it?

I'll merge anything that passes tests ;)

Gotta enable the workflows first (and as I said one Aqua test will fail).

You'll be able to see a docs preview at https://rafaqz.github.io/Interfaces.jl/previews/PR17 once PR CI is done

rafaqz commented 9 months ago

For ambiguity we just need to define this function:

test(::Type{<:Interfaces.Interface{Keys}}, ::Type, ::Interfaces.TestObjectWrapper)

The kwcall version we will get for free with it.

I've only used Literate.jl for making jupyter notebooks from scripts.

gdalle commented 9 months ago

For ambiguity we just need to define this function:

Can you maybe add that line? I'm not sure what to do with the Keys, T and T1 in particular

I've only used Literate.jl for making jupyter notebooks from scripts.

The principle here is the same, except that Literate.markdown turns a .jl with the Markdown in comments into a .md that can be parsed by Documenter

codecov-commenter commented 9 months ago

Codecov Report

Merging #17 (0b46f2a) into main (e793622) will decrease coverage by 3.31%. The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
- Coverage   78.76%   75.45%   -3.31%     
==========================================
  Files           4        4              
  Lines         113      110       -3     
==========================================
- Hits           89       83       -6     
- Misses         24       27       +3     
Files Coverage Δ
src/Interfaces.jl 100.00% <ø> (ø)

... and 2 files with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

gdalle commented 9 months ago

@rafaqz the docs preview apparently doesn't work because I opened this PR from a fork (since I'm not a repo member). But I tested it locally and it looks good. Also merged the changes from #19 so it's ready to roll