-
Notifications
You must be signed in to change notification settings - Fork 71
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
Doc custom style guide #846
Doc custom style guide #846
Conversation
14847b5
to
44dcbc2
Compare
Codecov Report
@@ Coverage Diff @@
## master #846 +/- ##
=======================================
Coverage 90.02% 90.02%
=======================================
Files 47 47
Lines 2486 2486
=======================================
Hits 2238 2238
Misses 248 248
Continue to review full report at Codecov.
|
This is how benchmark results would change (along with a 95% confidence interval in relative change) if e01c33f is merged into master:
Further explanation regarding interpretation and methodology can be found in the documentation. |
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 added some comments as a thank you for answering my issue especially as the answer was in the vignette I was reading 😁 !
@@ -0,0 +1,73 @@ | |||
--- | |||
title: "distribute_custom_style_guide" |
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.
Placeholder title which you might already know :-)
There are two packages that implement a third-party style guide: | ||
|
||
* [lorenzwalthert/oneliner](https://github.com/lorenzwalthert/oneliner) | ||
* [mlr-org/styler.mlr](https://github.com/mlr-org/styler.mlr) |
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.
Broken link
This implies that they won't make it to CRAN because packages calling private | ||
methods from other packages don't pass CRAN checks. The only way around this | ||
would be to export some styler internals, e.g. via a {styler.infra} package, | ||
but that would be a lot of work on our side. Another alternative for |
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.
maybe add that it's work you don't plan for now?
tidyverse style guide, use `styler::style_pkg()`, if you want to use a | ||
third-party style guide, use the other namespace, e.g. `styler.mlr::style_pkg()` | ||
* depend on {styler} and use styler internals via `:::`. | ||
This implies that they won't make it to CRAN because packages calling private |
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.
and that they're "fragile"?
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.e. if you do that, you should probably follow styler's development closely?
Co-authored-by: Maëlle Salmon <maelle.salmon@yahoo.se>
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 9869a14 is merged into master:
Further explanation regarding interpretation and methodology can be found in the documentation. |
This is how benchmark results would change (along with a 95% confidence interval in relative change) if 1a8906f is merged into master:
Further explanation regarding interpretation and methodology can be found in the documentation. |
Closes #721.