JuliaDiff / DiffRules.jl

A simple shared suite of common derivative definitions
Other
74 stars 38 forks source link

Increase Julia compat 1.3 #77

Closed cossio closed 2 years ago

cossio commented 2 years ago

Increase Julia compat bound to 1.3. This is needed for https://github.com/JuliaDiff/DiffRules.jl/pull/74.

Add CI testing on 1.3 (for consistency with compat) and 1.6 (LTS).

codecov-commenter commented 2 years ago

Codecov Report

Merging #77 (c3e6ab3) into master (f46977e) will increase coverage by 1.16%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #77      +/-   ##
==========================================
+ Coverage   95.74%   96.91%   +1.16%     
==========================================
  Files           2        3       +1     
  Lines         141      162      +21     
==========================================
+ Hits          135      157      +22     
+ Misses          6        5       -1     
Impacted Files Coverage Δ
src/rules.jl 100.00% <100.00%> (+0.76%) :arrow_up:
src/DiffRules.jl 100.00% <0.00%> (ø)
src/api.jl 84.37% <0.00%> (+29.82%) :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 f46977e...c3e6ab3. Read the comment docs.

mcabbott commented 2 years ago

Needs version bump, ideally to 1.7, after #76.

cossio commented 2 years ago

Needs version bump, ideally to 1.7, after #76.

Why do we need a new version when changing a dependency compat bound? This is not implying any change in the API of DiffRules. So why should we increase version here?

mcabbott commented 2 years ago

Every release needs a new version. Not a breaking version, so it's more or less free.

And I think it's meant to be a minor version, not patch, in case you ever need to release a bugfix for an older version (although this is unlikely).

cossio commented 2 years ago

Every release needs a new version. Not a breaking version, so it's more or less free.

And I think it's meant to be a minor version, not patch, in case you ever need to release a bugfix for an older version (although this is unlikely).

Ok. But which PRs are you planning to tag as releases? Maybe we can bundle them together and just do one release? Since you are already bumping version in #76, I don't need to do it here. DO you agree?

cossio commented 2 years ago

Every release needs a new version. Not a breaking version, so it's more or less free.

And I think it's meant to be a minor version, not patch, in case you ever need to release a bugfix for an older version (although this is unlikely).

Ah ok, I think I get it. I bumped the minor version, and rebased. This should be ready to go (as soon as tests pass).

cossio commented 2 years ago

The CI on Julia 1.7 is failing on Base.mod, by the way. And it seems to be a fault of the finitediff function used to test (I think the rule itself is fine). But that should be addressed in a separate PR. Can we merge this?

cossio commented 2 years ago

Regarding the CI test on Julia v.17, see #78

cossio commented 2 years ago

Can you also get rid of the branch in

Done

cossio commented 2 years ago

Tests pass. @mcabbott do you agree to merge this?