Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate old checkpointing syntax #17

Merged
merged 4 commits into from
May 10, 2021
Merged

deprecate old checkpointing syntax #17

merged 4 commits into from
May 10, 2021

Conversation

mzgubic
Copy link
Contributor

@mzgubic mzgubic commented May 10, 2021

follow up to #15

@codecov
Copy link

codecov bot commented May 10, 2021

Codecov Report

Merging #17 (84ac8a4) into main (8b048b6) will increase coverage by 1.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
+ Coverage   93.90%   95.09%   +1.19%     
==========================================
  Files           3        4       +1     
  Lines          82      102      +20     
==========================================
+ Hits           77       97      +20     
  Misses          5        5              
Impacted Files Coverage Δ
src/Checkpoints.jl 91.42% <100.00%> (+2.14%) ⬆️
src/deprecated.jl 100.00% <100.00%> (ø)
src/handler.jl 93.54% <100.00%> (+0.95%) ⬆️
src/session.jl 100.00% <100.00%> (ø)

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 8b048b6...84ac8a4. Read the comment docs.

@@ -0,0 +1,10 @@
function checkpoint_deprecation()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we did

Suggested change
function checkpoint_deprecation()
function checkpoint_deprecation(tags...)

then we we can move the isempty here.
and can print the alternative.

src/Checkpoints.jl Outdated Show resolved Hide resolved
@testset "deprecated tags syntax" begin
mktempdir() do path
Checkpoints.config("TestPkg.deprecated", path)
TestPkg.deprecated_checkpoint_syntax()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use @test_deprecated to check this, and also maybe surpress the warning.
I haven't tried yet

Suggested change
TestPkg.deprecated_checkpoint_syntax()
@test_deprecated TestPkg.deprecated_checkpoint_syntax()

Copy link
Member

@oxinabox oxinabox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mzgubic mzgubic merged commit e6c698b into main May 10, 2021
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