-
Notifications
You must be signed in to change notification settings - Fork 27
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
Organize parameter names v2 #582
Conversation
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
Before I start review, is this duplicated with #562? What is the difference? |
With quick look, everything seems ok, Can you still run BiocChec::BiocCheck() and ensure that lines that are modified in this PR follows Bioconductor guidelines? (If you did not do that already) I will check this PR as soon as possible |
BiocChec::BiocCheck() has no errors and three warnings. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## devel #582 +/- ##
========================================
Coverage ? 67.37%
========================================
Files ? 41
Lines ? 4928
Branches ? 0
========================================
Hits ? 3320
Misses ? 1608
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Thanks! |
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.
Ready for your review @antagomir
MARGIN issue will be fixed in different PR #578 |
Let's wait Leo's thoughts |
I might be missing something, is there a specific question or is this about general code review? |
General code review. This is affecting the whole system so good to have second reviewer |
done |
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
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.
I ran the old unit tests, and found out that some old parameter names were removed. With these changes everything should be good
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
Signed-off-by: Daena Rys <rysdaena8@gmail.com>
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.
Ready!
Ping #579 .