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

Add unified interface test utils #19

Merged
merged 7 commits into from
Sep 29, 2020
Merged

Add unified interface test utils #19

merged 7 commits into from
Sep 29, 2020

Conversation

sharanry
Copy link
Contributor

#18

Proposes a unified interface testing utilities similar to the ones added by @willtebbutt in JuliaGaussianProcesses/KernelFunctions.jl#159.

@codecov-commenter
Copy link

codecov-commenter commented Sep 28, 2020

Codecov Report

Merging #19 into master will increase coverage by 5.88%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
+ Coverage   70.58%   76.47%   +5.88%     
==========================================
  Files           5        5              
  Lines          17       17              
==========================================
+ Hits           12       13       +1     
+ Misses          5        4       -1     
Impacted Files Coverage Δ
src/likelihoods/gaussian.jl 71.42% <0.00%> (+14.28%) ⬆️

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 775140a...4297ff7. Read the comment docs.

Copy link
Member

@willtebbutt willtebbutt left a comment

Choose a reason for hiding this comment

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

This is looking good. Just a couple of style-related things.

We also need a patch version bump :)

test/test_utils.jl Outdated Show resolved Hide resolved
test/test_utils.jl Outdated Show resolved Hide resolved
test/test_utils.jl Outdated Show resolved Hide resolved
test/test_utils.jl Show resolved Hide resolved
sharanry and others added 2 commits September 29, 2020 10:12
@sharanry
Copy link
Contributor Author

We also need a patch version bump :)

Since we haven't registered the package and we aren't making any releases yet, are the patch bumps useful?

Also should we register once this is merged?

@theogf
Copy link
Member

theogf commented Sep 29, 2020

Haha no patch bump, I think @willtebbutt just forgot it was not registered! We need to leave it at 0.1.0 or it will not be automatically merged

@willtebbutt
Copy link
Member

🤦 @theogf is exactly correct - I keep forgetting that this package isn't registered yet...

@sharanry
Copy link
Contributor Author

facepalm @theogf is exactly correct - I keep forgetting that this package isn't registered yet...

Me too 😆. Should we merge this and register now that we set up the tests?

@willtebbutt
Copy link
Member

Sounds good to me.

@sharanry sharanry merged commit c6ef433 into master Sep 29, 2020
@sharanry sharanry deleted the sharan/test-utils branch September 29, 2020 08:46
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.

4 participants