Skip to content
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

Use cli messaging for cache #1127

Merged
merged 3 commits into from
Jun 3, 2023
Merged

Use cli messaging for cache #1127

merged 3 commits into from
Jun 3, 2023

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Jun 2, 2023

Add a link for RStudio users

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

This is how benchmark results would change (along with a 95% confidence interval in relative change) if e8836fd is merged into main:

  •   :ballot_box_with_check:cache_applying: 237ms -> 237ms [-1.86%, +2.06%]
  •   :ballot_box_with_check:cache_recording: 806ms -> 813ms [-0.17%, +1.87%]
  •   :ballot_box_with_check:without_cache: 1.54s -> 1.54s [-0.52%, +0.61%]

Further explanation regarding interpretation and methodology can be found in the documentation.

@lorenzwalthert
Copy link
Collaborator

Looks good, thanks. If you want, you could add links to a few more places:

  • full text search for help(
  • full text search for `?

Also, if you search for https://github.com/, you find a few more places where we can use the .internal option for error and warnings.

* Fix with autolinking in vignette
* Update a test that I broke.
@olivroy
Copy link
Contributor Author

olivroy commented Jun 3, 2023

I fixed a test.
However, I didn't find more places where .internal = TRUE would make sense.
The only place was at

"https://github.com/r-lib/styler and provide a reproducible example",
,

but it wouldn't really fit in the context.

@codecov-commenter
Copy link

Codecov Report

Merging #1127 (4356de7) into main (e8836fd) will not change coverage.
The diff coverage is 66.66%.

❗ Current head 4356de7 differs from pull request most recent head 015874f. Consider uploading reports for the commit 015874f to get more accurate results

@@           Coverage Diff           @@
##             main    #1127   +/-   ##
=======================================
  Coverage   92.18%   92.18%           
=======================================
  Files          46       46           
  Lines        2650     2650           
=======================================
  Hits         2443     2443           
  Misses        207      207           
Impacted Files Coverage Δ
R/vertical.R 60.00% <0.00%> (ø)
R/set-assert-args.R 90.47% <80.00%> (ø)
R/stylerignore.R 100.00% <100.00%> (ø)

@github-actions
Copy link
Contributor

github-actions bot commented Jun 3, 2023

This is how benchmark results would change (along with a 95% confidence interval in relative change) if e8836fd is merged into main:

  •   :ballot_box_with_check:cache_applying: 264ms -> 264ms [-1.1%, +1.08%]
  •   :ballot_box_with_check:cache_recording: 869ms -> 869ms [-0.41%, +0.55%]
  •   :ballot_box_with_check:without_cache: 1.62s -> 1.62s [-0.46%, +0.33%]

Further explanation regarding interpretation and methodology can be found in the documentation.

Copy link
Collaborator

@lorenzwalthert lorenzwalthert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lorenzwalthert lorenzwalthert merged commit d71e2cc into r-lib:main Jun 3, 2023
@olivroy olivroy deleted the patch-1 branch June 3, 2023 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants