Skip to content

Conversation

MichaelChirico
Copy link
Contributor

Closes #1253.

Appreciate some guidance on how to sew this up into a complete PR.

@MichaelChirico
Copy link
Contributor Author

PS it might help to write something about fledge in the CONTRIBUTING guide, I am not sure how to approach ensuring there is a proper NEWS entry.

https://github.com/r-lib/styler/blob/main/CONTRIBUTING.md

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2025

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

  • ✔️cache_applying: 26.4ms -> 26.2ms [-3.84%, +2.43%]
  • ✔️cache_recording: 391ms -> 390ms [-0.75%, +0.54%]
  • ✔️without_cache: 913ms -> 911ms [-0.62%, +0.29%]

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

@codecov
Copy link

codecov bot commented Feb 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.64%. Comparing base (b18a1cb) to head (e7b1565).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1254   +/-   ##
=======================================
  Coverage   91.64%   91.64%           
=======================================
  Files          46       46           
  Lines        2656     2658    +2     
=======================================
+ Hits         2434     2436    +2     
  Misses        222      222           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Feb 8, 2025

@MichaelChirico thanks for the contribution. Actually I don't use {fledge} anymore. It did not help me to make my release workflow faster since it failed multiple times when I tried it and the commands were not transparent (enough) to me. Can you please remove the reference to the PR? It's not really the convention in this repo, I mostly use git blame to find the PR if I need to. I.e. I'll add a news bullet only when I draft a release.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2025

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

  • ✔️cache_applying: 25.8ms -> 25.9ms [-1.85%, +2.77%]
  • ✔️cache_recording: 376ms -> 377ms [-0.08%, +0.85%]
  • ✔️without_cache: 878ms -> 880ms [-0.06%, +0.65%]

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

@MichaelChirico
Copy link
Contributor Author

Can you please remove the reference to the PR?

Sorry, is this about {fledge} (which I found at the top of the NEWS when considering an entry), or the comment about the issue # in the test suite?

@lorenzwalthert
Copy link
Collaborator

Two separate things:

  • no need to worry about fledge and NEWS, I'll adapt before the in the next release. I'll remove the reference to fledge there too.
  • In the diff you introduced, please remove the references to the PR / issue number

@MichaelChirico
Copy link
Contributor Author

OK, done

@github-actions
Copy link
Contributor

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

  • ✔️cache_applying: 26.5ms -> 26.2ms [-3.01%, +0.96%]
  • ❗🐌cache_recording: 415ms -> 419ms [+0.22%, +1.36%]
  • ✔️without_cache: 968ms -> 969ms [-1.23%, +1.4%]

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

@lorenzwalthert
Copy link
Collaborator

Great, thanks @MichaelChirico

@lorenzwalthert lorenzwalthert merged commit 8b43fe2 into r-lib:main Feb 11, 2025
17 checks passed
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.

Strange styling suggestion

2 participants