-
-
Notifications
You must be signed in to change notification settings - Fork 9
readme updates #166
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
readme updates #166
Conversation
Code Coverage SummaryDiff against mainResults for commit: e97c0b7 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Signed-off-by: Marcin <133694481+m7pr@users.noreply.github.com>
|
Looks Good 👍 |
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 can't comment on code as it wasn't changed, but on vignettes/teal-data.Rmd it is using the .../main/... on the {teal} URLs instead of the latest-tag
Do you mind if I commit to the branch?
diff --git a/vignettes/teal-data.Rmd b/vignettes/teal-data.Rmd
index 67cdb47..32429a7 100644
--- a/vignettes/teal-data.Rmd
+++ b/vignettes/teal-data.Rmd
@@ -43,9 +43,9 @@ my_general_data <- teal_data(
The `teal.data` package provides many features to help specify your data:
-- [Specifying `ADaM` data](https://insightsengineering.github.io/teal/main/articles/including-adam-data-in-teal.html)
-- [Specifying `MultiAssayExperiment` data](https://insightsengineering.github.io/teal/main/articles/including-mae-data-in-teal.html)
-- [Specifying general relational data](https://insightsengineering.github.io/teal/main/articles/including-general-data-in-teal.html)
+- [Specifying `ADaM` data](https://insightsengineering.github.io/teal/latest-tag/articles/including-adam-data-in-teal.html)
+- [Specifying `MultiAssayExperiment` data](https://insightsengineering.github.io/teal/latest-tag/articles/including-mae-data-in-teal.html)
+- [Specifying general relational data](https://insightsengineering.github.io/teal/latest-tag/articles/including-general-data-in-teal.html)
- [Specifying relationships between your datasets](join-keys.html)
- [Dynamically loading your data (`DDL`)](using-delayed-data-basic.html)
- [Pre-processing data](preprocessing-data.html)|
Thanks for noticing! Fixed in here 07f4a36 |
donyunardi
left a comment
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.
LGTM
donyunardi
left a comment
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 urlchecker::url_check() and got this message:
✖ Error: man/col_labels.Rd:8:7 Error: CRAN URL not in canonical form
\href{https://cran.r-project.org/web/packages/formatters/index.html}{formatters} package, to reduce the complexity of
✖ Error: man/col_relabel.Rd:8:7 Error: CRAN URL not in canonical form
\href{https://cran.r-project.org/web/packages/formatters/index.html}{formatters} package, to reduce the complexity of
Should we update the formatters URL in the roxygen note for these two functions to:
\href{https://cran.r-project.org/package=formatters}{formatters}
averissimo
left a comment
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.
LGTM after addressing @donyunardi's comment about urls
|
Thanks @donyunardi - really good finding! Thanks for double checking |
|
Feedback addresses, waiting for @donyunardi to re-review and accept! |
donyunardi
left a comment
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.
LGTM!
Related to https://github.com/insightsengineering/coredev-tasks/issues/462