search

r-lib / styler

Non-invasive pretty printing of R code
https://styler.r-lib.org
Other
725 stars 71 forks source link

Add a script for generating many roxygen-examples-complete tests, apply it #1111

Closed MichaelChirico closed 10 months ago

MichaelChirico commented 1 year ago

Closes #1105. Note that this includes #1110 so the diff is too large for now. I couldn't figure out a way to specify that commit as the diffbase while still creating this PR on r-lib/styler (since that commit is on my fork). Sorry for the visual noise.

lorenzwalthert commented 1 year ago

1110 is merged, so I am going to drop this from the title. Feel free to rebase.

lorenzwalthert commented 1 year ago

@IndrajeetPatil since you started to review, I leave this to you.

lorenzwalthert commented 1 year ago

@MichaelChirico due to #1119, we need to release a new version of {styler} soon (mid next week), just if you want this PR to be included. I don't think we'll have another release soon after that.

MichaelChirico commented 1 year ago

Thanks for the ping, feedback handled.

codecov-commenter commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (2218546) 92.31% compared to head (5ab3a30) 92.31%. Report is 3 commits behind head on main.

:exclamation: Current head 5ab3a30 differs from pull request most recent head 05c76f4. Consider uploading reports for the commit 05c76f4 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1111 +/- ## ======================================= Coverage 92.31% 92.31% ======================================= Files 46 46 Lines 2655 2655 ======================================= Hits 2451 2451 Misses 204 204 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lorenzwalthert commented 1 year ago

Thanks. @IndrajeetPatil I leave the review to you, only thing I would add is that I am not a big fan of adding a top level directory (albeit hidden) to the repo. Or do we expect future scripts to live in .dev? Can't we just place this script in testthat/? IIRC, files that don't start with test- or helper- are not executed when {testthat} tests are run.

IndrajeetPatil commented 10 months ago

The precommit workflow is going to fail until https://github.com/r-lib/styler/pull/1158 is merged, and so need to wait for that PR to be merged.

lorenzwalthert commented 10 months ago

Since we moved the script to generate the tests, some code comments still point to old directory…