-
Notifications
You must be signed in to change notification settings - Fork 80
Fix sample obj #1074
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
Fix sample obj #1074
Conversation
#1073 was just merged, can you pull upstream on this? |
…fix-sample-obj
@squirrelo Done! |
👍 |
Looks good. @squirrelo @josenavas is there a way to test this, or are we merging everything into the |
@adamrp all the functionality is not in place after a couple more PR. I think the best path is to review the code to see if there is anything horribly wrong, and then wait for the PR that everything is in place to make sure that everything is correct. Is really hard to test until everything is in place, but if the code looks good, I can fix the remaining issues later. Does everybody agrees with that? |
@josenavas, yes that's a good strategy moving forward. |
great! Thanks. |
This PR is build on top of
#1072(merged) and #1073. At least #1072 should be merged in master before merging this one, so the commits from there (quite a few) disappear here.This PR fixes the BaseSample, Sample and PrepSample objects and their tests. The tests for these objects are expected to pass.