software-review icon indicating copy to clipboard operation
software-review copied to clipboard

octolog

Open assignUser opened this issue 2 years ago β€’ 33 comments

Reviewers:

Submitting Author Name: Jacob Wujciak-Jens Submitting Author Github Handle: @assignUser Repository: https://github.com/assignUser/octolog/ Version submitted: Submission type: Standard Editor: @annakrystalli Reviewers: @pat-s, @annakrystalli

Due date for @pat-s: 2022-03-30

Due date for @annakrystalli: 2022-06-07 Archive: TBD Version accepted: TBD Language: en

  • Paste the full DESCRIPTION file inside a code block below:
Package: octolog
Title: Better 'Github Action' Logging
Version: 0.1.1.9000
Authors@R: 
    person("Jacob", "Wujciak-Jens", , "[email protected]", role = c("aut", "cre"),
           comment = c(ORCID = "0000-0002-7281-3989"))
Description: Provides an API for 'Github' <https://github.com> workflow
    commands and makes it possible to signal conditions from within R that
    will create annotations when used within 'Github Actions'
    <https://github.com/features/actions> but raise normal R conditions
    when used interactively.
License: MIT + file LICENSE
URL: https://github.com/assignUser/octolog,
    https://assignuser.github.io/octolog/
BugReports: https://github.com/assignUser/octolog/issues
Imports: 
    cli (>= 3.0.0),
    fs,
    glue,
    magrittr,
    openssl,
    rlang (>= 1.0.0),
    utils,
    withr
Suggests: 
    knitr,
    covr,
    mockery (>= 0.4.2.9000),
    testthat (>= 3.0.0),
    rmarkdown
Remotes: 
    r-lib/mockery
Config/testthat/edition: 3
Config/testthat/parallel: true
Encoding: UTF-8
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.1.2
VignetteBuilder: knitr

Scope

  • Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

    • [ ] data retrieval
    • [ ] data extraction
    • [ ] data munging
    • [ ] data deposition
    • [x] workflow automation
    • [ ] version control
    • [ ] citation management and bibliometrics
    • [ ] scientific software wrappers
    • [ ] field and lab reproducibility tools
    • [ ] database software bindings
    • [ ] geospatial data
    • [ ] text analysis
  • Explain how and why the package falls under these categories (briefly, 1-2 sentences): Octolog aims to make it easier for R author's to use the full power of Github Actions as a CI service.

  • Who is the target audience and what are scientific applications of this package? Using CI is recognized as an important part in the development process of (scientific) R software and octlog aims to make this easier and more powerful.

  • Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?

As far as I know there are none.

  • (If applicable) Does your package comply with our guidance around Ethics, Data Privacy and Human Subjects Research?

  • If you made a pre-submission inquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted. https://github.com/ropensci/software-review/issues/501

  • Explain reasons for any pkgcheck items which your package is unable to pass.

Technical checks

Confirm each of the following by checking the box.

This package:

Publication options

  • [x] Do you intend for this package to go on CRAN?

  • [ ] Do you intend for this package to go on Bioconductor?

  • [ ] Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

MEE Options
  • [ ] The package is novel and will be of interest to the broad readership of the journal.
  • [ ] The manuscript describing the package is no longer than 3000 words.
  • [ ] You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
  • (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
  • (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
  • (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

  • [x] I agree to abide by rOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

assignUser avatar Feb 02 '22 20:02 assignUser

Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type @ropensci-review-bot help for help.

ropensci-review-bot avatar Feb 02 '22 20:02 ropensci-review-bot

:rocket:

Editor check started

:wave:

ropensci-review-bot avatar Feb 02 '22 20:02 ropensci-review-bot

Checks for octolog (v0.1.1.9000)

git hash: 8c283f19

  • :heavy_check_mark: Package name is available
  • :heavy_check_mark: has a 'CITATION' file.
  • :heavy_check_mark: has a 'codemeta.json' file.
  • :heavy_check_mark: has a 'contributing' file.
  • :heavy_check_mark: uses 'roxygen2'.
  • :heavy_check_mark: 'DESCRIPTION' has a URL field.
  • :heavy_check_mark: 'DESCRIPTION' has a BugReports field.
  • :heavy_check_mark: Package has at least one HTML vignette
  • :heavy_check_mark: All functions have examples.
  • :heavy_check_mark: Package has continuous integration checks.
  • :heavy_check_mark: Package coverage is 94.6%.
  • :heavy_check_mark: R CMD check found no errors.
  • :heavy_check_mark: R CMD check found no warnings.

Package License: MIT + file LICENSE


1. Statistical Properties

This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.

Details of statistical properties (click to open)

The package has:

  • code in R (100% in 4 files) and
  • 1 authors
  • 1 vignette
  • no internal data file
  • 8 imported packages
  • 20 exported functions (median 8 lines of code)
  • 30 non-exported functions in R (median 9 lines of code)

Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages The following terminology is used:

  • loc = "Lines of Code"
  • fn = "function"
  • exp/not_exp = exported / not exported

The final measure (fn_call_network_size) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile.

measure value percentile noteworthy
files_R 4 28.3
files_vignettes 1 68.4
files_tests 4 79.0
loc_R 242 27.0
loc_vignettes 26 3.4 TRUE
loc_tests 185 53.1
num_vignettes 1 64.8
n_fns_r 50 56.6
n_fns_r_exported 20 67.4
n_fns_r_not_exported 30 52.8
n_fns_per_file_r 9 84.2
num_params_per_fn 2 10.4
loc_per_fn_r 9 24.3
loc_per_fn_r_exp 8 18.6
loc_per_fn_r_not_exp 9 27.1
rel_whitespace_R 26 40.1
rel_whitespace_vignettes 81 14.2
rel_whitespace_tests 21 50.9
doclines_per_fn_exp 25 23.5
doclines_per_fn_not_exp 0 0.0 TRUE
fn_call_network_size 38 60.5

1a. Network visualisation

Click to see the interactive network visualisation of calls between objects in package


2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

3a. Continuous Integration Badges

R-CMD-check pkgcheck.yaml

GitHub Workflow Results

name conclusion sha date
Octolog example workflow success 8c283f 2022-02-02
pages build and deployment success 9d6c29 2022-02-02
pkgcheck success 8c283f 2022-02-02
pkgdown success 8c283f 2022-02-02
R-CMD-check success 8c283f 2022-02-02
test-coverage success 8c283f 2022-02-02

3b. goodpractice results

R CMD check with rcmdcheck

rcmdcheck found no errors, warnings, or notes

Test coverage with covr

Package coverage: 94.57

Cyclocomplexity with cyclocomp

No functions have cyclocomplexity >= 15

Static code analyses with lintr

lintr found the following 17 potential issues:

message number of times
Avoid changing the working directory, or restore it in on.exit 2
Lines should not be more than 80 characters. 15

Package Versions

package version
pkgstats 0.0.3.88
pkgcheck 0.0.2.227

Editor-in-Chief Instructions:

This package is in top shape and may be passed on to a handling editor

ropensci-review-bot avatar Feb 02 '22 20:02 ropensci-review-bot

@ropensci-review-bot assign @annakrystalli as editor

jooolia avatar Feb 08 '22 13:02 jooolia

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@ropensci-review-bot help

ropensci-review-bot avatar Feb 08 '22 13:02 ropensci-review-bot

@ropensci-review-bot assign @annakrystalli as editor

jooolia avatar Feb 08 '22 13:02 jooolia

Assigned! @annakrystalli is now the editor

ropensci-review-bot avatar Feb 08 '22 13:02 ropensci-review-bot

Thank you @jooolia for your work so far!

I just wanted to note that my test coverage as displayed by {pkgcheck} has seemingly dropped below 75% but this is due to an issue with {covr} and the way it handles expect_snapshot tests: https://github.com/r-lib/covr/issues/482 The actual coverage is >90%: https://app.codecov.io/gh/assignUser/octolog

Edit: looks like the issue only happens with running {pkgcheck}/{covr} locally and using the gh-action as the result reported by the review bot is accurate.

assignUser avatar Feb 08 '22 18:02 assignUser

Hello @assignUser and many thanks for your package submission! πŸ‘‹

I'll be your editor and will start editor checks shortly and report back. πŸ‘

annakrystalli avatar Feb 09 '22 16:02 annakrystalli

Hey @annakrystalli do you need any input from my side at this point to continue the submission?

assignUser avatar Mar 08 '22 12:03 assignUser

Editor checks:

  • [ ] Documentation: The package has sufficient documentation available online (README, pkgdown docs) to allow for an assessment of functionality and scope without installing the package. In particular,
    • [ ] Is the case for the package well made?
    • [ ] Is the reference index page clear (grouped by topic if necessary)?
    • [ ] Are vignettes readable, sufficiently detailed and not just perfunctory?
  • [x] Fit: The package meets criteria for fit and overlap.
  • [x] Installation instructions: Are installation instructions clear enough for human users?
  • [x] Tests: If the package has some interactivity / HTTP / plot production etc. are the tests using state-of-the-art tooling?
  • [x] Contributing information: Is the documentation for contribution clear enough e.g. tokens for tests, playgrounds?
  • [x] License: The package has a CRAN or OSI accepted license.
  • [x] Project management: Are the issue and PR trackers in a good shape, e.g. are there outstanding bugs, is it clear when feature requests are meant to be tackled?

Editor comments


Hello @assignUser! and many apologies for the delayed response. I had been battling with my compilers since I upgraded my laptop and I finally managed to get them sorted and run all checks and tests locally!

The package looks in good shape. There are a couple of very minor issues flagged up below. My main comment would be regarding documentation. I feel there is a little too much linking to outside resources, or to examples that don't clearly walk the user through how package functionality is implemented but rather leave them to have to figure it out.

My personal suggestion (that would have helped me understand the package more easily) would be:

  1. To include the Introduction to octolog (first) section on the Getting Started vignette in the README (so users can get a flavour of functionality from the README itself). See the README & Documentation sections in the rOpenSci dev guide for details, in particular this:

The README, the top-level package docs, vignettes, websites, etc., should all have enough information at the beginning to get a high-level overview of the package and the services/data it connects to, and provide navigation to other relevant pieces of documentation. This is to follow the principle of multiple points of entry i.e. to take into account the fact that any piece of documentation may be the first encounter the user has with the package and/or the tool/data it wraps.

  1. When demonstrating package usage, I feel it would be better to use a more real world example of usage. Currently you are demonstrating code in a .github/workflows/R/conditions.R file, not a typical place a user would expect to run code from, and don't show explicitly how the code is run through GitHub Actions. While ok for a quick demo perhaps in a README, I feel you a deeper dive is required in the vignettes. In there, it would be more useful to show how the code would run in a small demo package, either as part of a function or a test for example.

  2. To have a separate vignette for the workflow commands that includes examples.

  3. You could even consider splitting of the security chapter to a separate vignette if you wanted.

Lastly, the link to the docs in the README returns 404.

Minor issues:

A few things flagged up by goodpractice::gp()


── GP octolog ─────────────────────────────────────────────────────────────────────────────

It is good practice to

  βœ– avoid long code lines, it is bad for readability. Also, many people
    prefer editor windows that are about 80 characters wide. Try make your lines
    shorter than 80 characters

    R/commands.R:4:1
    R/commands.R:12:1
    R/commands.R:37:1
    R/commands.R:47:1
    R/commands.R:90:1
    ... and 9 more lines

  βœ– avoid calling setwd(), it changes the global environment. If you
    need it, consider using on.exit() to restore the working directory.

    tests/testthat/test-util.R:49:16
    tests/testthat/test-util.R:51:3
─────────────────────────────────────────────────────────────────────────────────────────── 

I will get on looking for reviewers straight away. Let me know what you think about the documentation comments. We could also wait to get the opinions of the reviewers.

annakrystalli avatar Mar 08 '22 15:03 annakrystalli

@ropensci-review-bot assign @pat-s as reviewer

annakrystalli avatar Mar 09 '22 11:03 annakrystalli

@pat-s added to the reviewers list. Review due date is 2022-03-30. Thanks @pat-s for accepting to review! Please refer to our reviewer guide.

ropensci-review-bot avatar Mar 09 '22 11:03 ropensci-review-bot

@pat-s: If you haven't done so, please fill this form for us to update our reviewers records.

ropensci-review-bot avatar Mar 09 '22 11:03 ropensci-review-bot

@annakrystalli Thanks for the feedback. I'll start to think about how to better show case it but I would also be interested in the reviewers opinions :)

assignUser avatar Mar 09 '22 15:03 assignUser

@assignUser Sounds good to me!

annakrystalli avatar Mar 09 '22 19:03 annakrystalli

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • Briefly describe any working relationship you have (had) with the package authors.
  • [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (if you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • [x] A statement of need: clearly stating problems the software is designed to solve and its target audience in README
  • [x] Installation instructions: for the development version of package and any non-standard dependencies in README
  • [x] Vignette(s): demonstrating major functionality that runs successfully locally
  • [x] Function Documentation: for all exported functions
  • [ ] Examples: (that run successfully locally) for all exported functions
  • [x] Community guidelines: including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

Functionality

  • [x] Installation: Installation succeeds as documented.
  • [ ] Functionality: Any functional claims of the software been confirmed.
  • [x] Performance: Any performance claims of the software been confirmed.
  • [x] Automated tests: Unit tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • [x] Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.

Estimated hours spent reviewing: 2,5

  • [x] Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

General

Overall I think the package is a useful contribution and can be of good use use in CI/CD workflows. It simplifies the combined usage of local and CI/CD execution. I will also try it out in combination with {tic} to better highlight and format log outputs! In fact, I've played around with it a bit in the octolog branch in {tic} (see CI runs) and the condition signaling worked as expected!

Please finde some comments below of points which could possibly be improved/changed to make the package even better :)

I think the motivation and intro could profit from one or two sentences more highlighting the scope of the package and how {octolog} might improve the experience of a package author/analyst who works with GHA. I think the word "additionally" suggests that the condition signaling in R is just a "nice side effect" whereas it is actually one of the main points. To be clear: I think this sentence just needs a slight rewrite and the word "additionally" should probably be avoided to not "value down" this part.

I was wondering where the name {octolog} originates from? I couldn't find a statement about it - maybe this could be added? (I assume it originates from the "octo" being GH logo animal - but maybe I am wrong? :)

Dependencies

I know this usually is a hot debate with many different views but let me share mine :) I see that there is a strong focus on tidyverse packages (which is not a problem per se and tidyverse packages are of course well programmed packages), yet they usually bring in additional dependencies and one has to ask themselves if it's worth it - especially on the R package side (scripting/data analysis is a different use case). Please don't see this is a "blocker" or similar, just as a personal opinion or "food for thought" :)

I think the package could be substantially reduced WRT to package dependencies:

  • Often the magrittr pipe is only used in scripting/analysis and not within packages as it can be easily replaced

  • {rlang} is similar to {magrittr} state above, though I see that it might do some things "better" than base R checks etc. I see that mainly rlang::trace_back() is used and one call to rlang::exec(). I haven't yet made use of the former but wanted to ask which additional benefits the use of rlang::trace_back() brings compared to traceback() WRT to the package functionality?

  • I like to use {fs} in scripting but try to avoid in packages due to it's dependency on libuv. This is especially costly and (possibly annoying for unexperienced users) in CI as the system dependency first needs to be installed. This costs time during every run and I think the {fs} dependency could be easily avoided.

Avoiding all of the above would leave the package with {cli}, {glue} and {withr} as direct dependencies (in IMPORTS) which would be an improvement to me.

README

  • The initial example and screenshot is somewhat hard to understand for first time users. To make it fully clear I'd suggest to also add an example how the error msg would look like in an interactive example - maybe even side by side in a a table?

  • Installation: Given that {pak} is still quite unstable and non-standard WRT to package installation (and is a third-party package), I'd not use it as the "default" installation method. I'd suggest to use install.packages() and mention {pak} as an alternative. In addition, remotes:: should be used instead of devtools::

  • The following URL in the README leads to a 404: website

  • Footnote 2 states that GHA is "only free for public repos". It is also free for private repos but with a limited amount of usage (in mins)

Vignettes

Get Started

  • The vignette indicates that {octolog} only works with {cli} message output and not with normal message() or warning() calls. If so, this should be highlighted more prominently at the start of the vignette or in the README

  • The example on octo_abort() should maybe get it's own section. Especially because "This can be useful if you want a workflow to fail (e.g.Β to block a PR) by throwing an error but do not want to stop the R process." is one of the selling features of {octolog}

  • The "masking" section could profit from an additional R code block showcasing the R code directly as the screenshot is somewhat hard to read. Also it might be of interested to note why these two functions differ and which one should be used in which scenario.

  • The "stopping command parsing" section could profit from a toy example. I find it quite hard to image a scenario in which the describe command could be of good use. The last two sentences "This is not intended to protect against other R code. Due to R's metaprogramming capabilities it is not possible to protect the token within the same R session." are not fully clear to me in this context. I think this section could profit from more context and a clear example :)

Functions

  • https://jacob.wujciak.de/octolog/reference/enable_github_colors.html : Examples throw an error

  • Maybe group the pkgdown reference output in meaningful categories?

  • octo_add_path() : the description is unclear to me

  • octo_mask_value(): examples error

  • octo_start_group(): The example errors. I think this could be worth getting it's own section in a vignette?

  • octocat(): what is it's relation to the other cli-based functions?

Stylistic

  • Github -> GitHub

  • It would be great to somehow differentiate the package from normal text, either by formatting is as code or wrapping it in brackets such as {octolog}

  • Typos:

    • README: powerful,free (whitespace)

    • Get started vignette: using octo_stop_commands.Internally the function

  • Screenshots: Could possibly be of higher quality

pat-s avatar Mar 14 '22 17:03 pat-s

Thanks @pat-s for your review! @assignUser I'm still looking for a second reviewer so I suggest holding out responding or making changes (at least not in the main branch) until both reviews are complete.

annakrystalli avatar Mar 14 '22 18:03 annakrystalli

Thank you for your helpful review @pat-s ! I will implement/address your points after the second review as suggested by @annakrystalli

assignUser avatar Mar 14 '22 18:03 assignUser

Dear @assignUser, Many apologies for the delay in the review. I've had a lot of trouble finding a second reviewer (and in between have also moved countries!). I'm settled now though so to get things moving again and given I have no conflict of interest with the package, I'll go ahead an volunteer as a reviewer. ☺️

annakrystalli avatar May 17 '22 08:05 annakrystalli

@ropensci-review-bot assign @annakrystalli as reviewer

annakrystalli avatar May 17 '22 08:05 annakrystalli

@annakrystalli added to the reviewers list. Review due date is 2022-06-07. Thanks @annakrystalli for accepting to review! Please refer to our reviewer guide.

ropensci-review-bot avatar May 17 '22 08:05 ropensci-review-bot

@annakrystalli: If you haven't done so, please fill this form for us to update our reviewers records.

ropensci-review-bot avatar May 17 '22 08:05 ropensci-review-bot

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

  • Briefly describe any working relationship you have (had) with the package authors.
  • [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (if you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

  • [x] A statement of need: clearly stating problems the software is designed to solve and its target audience in README
  • [x] Installation instructions: for the development version of package and any non-standard dependencies in README
  • [x] Vignette(s): demonstrating major functionality that runs successfully locally
  • [x] Function Documentation: for all exported functions
  • [ ] Examples: (that run successfully locally) for all exported functions
  • [x] Community guidelines: including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

Functionality

  • [x] Installation: Installation succeeds as documented.
  • [x] Functionality: Any functional claims of the software been confirmed.
  • [x] Performance: Any performance claims of the software been confirmed.
  • [x] Automated tests: Unit tests cover essential functions of the package and a reasonable range of inputs and conditions. All tests pass on the local machine.
  • [x] Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.

Estimated hours spent reviewing: 4

  • [x] Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

Review Comments

Fistly many apologies for the delays in getting the package through the review process @assignUser. As for the package, given how popular GitHub Actions are becoming (and it's great to hear it works nicely for tic too), it's a nice and clean addition to packages in the R continuous integration space.

Thank you as well again @pat-s for your excellent review. I pretty much agree with all of @pat-s's comments, especially the one on streamlining dependencies. I think the suggestion is a smart one and second it.

So as not to repeat the same points, I focus on a few additional comments I have, primarily building on my initial points in the editor's checks as well as some mentioned by @pat-s regarding documentation.

Documentation:

Minor points
  • In the pgkdown docs, there are many errors in examples of functions with the error could not find function <function_name>) e.g. enable_github_colors, encode_string, octo_warn and more (some of which run locally).

  • When running examples locally, often Sys.setenv(GITHUB_ACTIONS = "true") needed to be run. It would be good if example code cleaned up after itself with withr::local_envvar(GITHUB_ACTIONS = "true") like you do in tests.

General package documentation

While I appreciate that you point to a live example of GA runs using octolog from the README / Getting started, I feel more explanation of what is going on would be appreciated by new users.

For example, for starters, I would explicitly link to the test-octolog.yaml workflow that is being run.

Then I would point the user to the lines in the workflow that are running the R code that are generating the messages by focusing on this section and explaining what is going on: https://github.com/assignUser/octolog/blob/8e71de9077f0f6a3e5fbefb78be5c6f32c0cbf47/.github/workflows/test-octolog.yaml#L34-L89

More specifically I might explain that it's sourcing /.github/workflows/R/conditions.R (which is so short I might even include it in the docs) and then running R code from the workflow yaml itself.

For me the key is to make clear not just the code that generates the output in GA, but also where they appear in R code in the repo and how they then get run through GitHub Actions

Two additional suggestions to make the usefulness of the package to the R user more concrete and easy to experiment with.

  1. While your examples in test-octolog.yaml are comprehensive with respect to the generic types of outputs the octolog API and your package offer, I feel they could be improved in terms of helping R users visualise why it would be useful to include them. It would be good to provide some more real life examples (especially more realistic R continuous integration scenarios) and explain some of the ways in which R code might be run in GA (e.g. raw R code in the workflow yaml, sourcing R code, code being run as part of package tests, package loading).

Perhaps you could set up a tiny R package and placing octolog code in actual function files in R. Messages could be deliberate error messages, package loading messages, package dependency checks, progress messages, basically whatever you think you might inspire users to visualise their use.

I don't want to be prescriptive here, I'm just trying to motivate some ideas. The important point of all of this is to give a better and broader view of how they might be useful in GitHub Actions to an R user.

  1. If you set up such a demo repository, you could even set it up as a template that users could copy (or if not just fork) and direct them to use it as a playground for testing out package functionality. This could get them started much quicker with experimenting with package functionality and for me greatly improve understanding of how the package works and how they might use it.

Minor points

good practice notes

There are still a couple of suggestions being picked up by goodpractice::gp(). I feel the ones about line length are not a huge deal but they still remain best practice for readability.

The note about using setwd(), again, I'm not sure it's a big deal because it's being used in tests and it seems and you clean up nicely with withr::defer(setwd(old_wd)). I'm guessing this is the best way to achieve what you're trying to do. If there's no other way, I think this will be fine.

spell check

Probably the only potentially valid typo is in the roxygen docs for enable_github_colors:

sideeffects         enable_github_colors.Rd:29

Conclusion

I know it might feel like my suggestions are overboard but I feel it's really important to consider the perspective of complete beginners, walk them through very explicitly how things work (instead of linking to examples with quite a few working parts that they need to reverse engineer) and help them visualise how the package can help them.

Very open to hearing your thoughts on this!

annakrystalli avatar Jun 08 '22 08:06 annakrystalli

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/502#issuecomment-1067080447 time 2.5

annakrystalli avatar Jun 08 '22 08:06 annakrystalli

Logged review for pat-s (hours: 2.5)

ropensci-review-bot avatar Jun 08 '22 08:06 ropensci-review-bot

@ropensci-review-bot https://github.com/ropensci/software-review/issues/502#issuecomment-1149620624 time 4

annakrystalli avatar Jun 08 '22 08:06 annakrystalli

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@ropensci-review-bot help

ropensci-review-bot avatar Jun 08 '22 08:06 ropensci-review-bot

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/502#issuecomment-1149620624 time 4

annakrystalli avatar Jun 08 '22 08:06 annakrystalli

Logged review for annakrystalli (hours: 4)

ropensci-review-bot avatar Jun 08 '22 08:06 ropensci-review-bot