-
-
Notifications
You must be signed in to change notification settings - Fork 9
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 reorder columns #256
Fix reorder columns #256
Conversation
✅ All contributors have signed the CLA |
…ering/tlg-catalog into 255_fix_column_order@main# Please enter a commit message to explain why this merge is necessary,
Signed-off-by: Davide Garolini <[email protected]>
I have read the CLA Document and I hereby sign the CLA |
recheck |
We might need to wait for #254 before the merge |
Unit Tests Summary340 tests 252 ✅ 1m 0s ⏱️ Results for commit 427c9bb. ♻️ This comment has been updated with latest results. |
Unit Test Performance DifferenceTest suite performance difference
Additional test case details
Results for commit 7dda884 ♻️ This comment has been updated with latest results. |
Hey @Melkiades there are a few snapshot errors due to spacing. Would you be able to upload the newest version? Then we will be having a green state of CI! |
package/tests/testthat/_snaps/stable/tables-efficacy-rbmit01.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…ering/tlg-catalog into 255_fix_column_order@main
Yes |
[ FAIL 0 | WARN 0 | SKIP 88 | PASS 253 ] I do not get any error on my local |
hi @Melkiades @pawelru , I am feeling we should add minimum version in the description for tlg-c |
I agree, as the tests are generated from within the package makes sense to me |
Plot tests are tricky. It's on our to-do list to investigate it further. It's fine to disable this but please consider this a temporary solution.
Is this not enough? tlg-catalog/package/tests/testthat/setup.R Lines 6 to 14 in 414372f
(feel free to enhance it to make it more clear for other devs)
This won't have any impact. Currently, there are two testing environments:
None of these will look into the minimal version supported. That should be probably a separate testing environment but let's leave it for now. |
This makes sense! thanks Pawel |
package/tests/testthat/_snaps/development/tables-efficacy-rbmit01.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amazing! looks good to me. Good call for dropping rbmi table. no one is using it atm
#255