About add_package_checks()

[pat-s]

The function is pretty magical as it does a lot of stuff in the background.

For non-experienced users it might lead to confusion whats going on actually.

Combined with the possible danger of specifying steps twice #68 (comment), one option would be to omit this function and explicitly place all its steps in the template?

If not, we should probably warn in prepare_all_stages if certain steps (e.g. build_pkgdown, codecov, update_packages) are defined multiple times?

[krlmlr]

Interesting. I thought most users could just use add_package_checks() in their tic.R, and that this would become the default for package checks. Would make it easier to update the strategy for package checks later too.

The private argument to add_package_checks() complicates matters. I'll see how to get rid of it, perhaps tic.R will need library(tic) but would become self-contained in the result.

[pat-s]

I thought most users could just use add_package_checks() in their tic.R, and that this would become the default for package checks. Would make it easier to update the strategy for package checks later too.

This is the big advantage of it, yes. I definitely see the point. But I also see the potential to cause confusion. If users want to change something in the default, they have to define all single steps and stages on their own taking care of not duplicating stuff. Or leaving add_package_checks() out completely.
Users unfamiliar with tic who read tic.R and see all the stages/steps stuff might think they know whats going on but will most likely miss add_package_checks() and will be confused where certain steps are coming from.

Even if we take care of the duplication problem, it will cause confusion which step of add_package_checks() is currently really active and which is only a duplicate.

Maybe it would be good to have some other views on it; @maelle maybe?

[krlmlr]

We need to communicate that users should use either use add_package_checks() or add_step(step_rcmdcheck()) etc., but not both. The semantics of add_package_checks() are documented in ?DSL , and the function's contents should look like it can be copy-pasted directly to a tic.R file for further editing (which it currently doesn't but will do).

[maelle]

It could be like goodpractice::gp that uses a default list of checks by default, but one can pass a different list?

[krlmlr]

Need to think about it a bit. I like the idea of check templates, which consist of adding one or more steps added to one or more stages. Let's break down add_package_checks().

[krlmlr]

The add_package_checks() macro got much leaner now, essentially only including step_rcmdcheck() call and a call to run coverage checks.

This function adds value, though: the user doesn't need to think about which stage(s) to add the step(s) to. I think the way forward here is to provide an add_rcmdcheck() and an add_covr() function and have add_package_checks() call these.

[pat-s]

The add_package_checks() macro got much leaner now, essentially only including step_rcmdcheck() call and a call to run coverage checks.

What about build_pkgdown()?

[krlmlr]

add_build_pkgdown() ?

[maelle]

A step involving lintr? cf r-lib/lintr#340 (comment)

[krlmlr]

As discussed today: TicStepWithPackageDeps should be a step (perhaps step_install_deps()), not a base class. The add_package_checks() macro includes this step, but it's not included by default with step_rcmdcheck().