define contribution roles for developers, reviewers, maintainers.

[avallecam]

I drafted steps for different roles in the contribution guidelines.

The goal would be to keep a homogeneous package environment among developers and reviews during review on a “development branch” different to the main.

As mentioned in #31 packages could be in continuous update, and tutorial development can try to manage that situation.

Summarized implications are:

• We will use sandpaper::use_package_cache() in the project.
• The developer will need to register the package version to use in a renv.lock file.
• The reviewer will need to restore the environment defined by the Developer.
• The maintainer will need to review how to automate package updates with a carpentries-bot in the main branch only.

Specific steps in:

• preview: https://github.com/epiverse-trace/tutorials/blob/main/CONTRIBUTING.md#contribution-roles
• or permalink:

  tutorials/CONTRIBUTING.md

  Line 74 in b207e3f

  ### Contribution roles

It would be helpful to have your feedback @amanda-minter as a developer, @CarmenTamayo @pratikunterwegs as reviewers, and @Bisaloo from your expertise with workflows, if I could make this clearer or change any step.

[pratikunterwegs]

Thanks @avallecam - I'm not very familiar with {sandpaper} and the way the Carpentries' Workbench is set up, or the intended workflow when developing training material with this template. Would it be useful for you, Amanda, or Hugo to give a brief introduction to it at one of the developer meetings?

Overall this plan seems fine and similar to the one we use for our other packages.

1. I would only add that it's the developers' responsibility to keep their branches up to date with main, as with Update sandpaper cache, update digest #31 once it's merged.
2. What is the policy around developers taking on dependencies for the lesson environment, or in general - how do/should these feed into main from a feature branch?

[avallecam]

Yep, a proper intro to sandpaper and the carpentries will help others to support us on this. We're looking to have this intro on Nov 7th.

Our intention is to go as similar as we can to the package workflows, so a comparison between both will be helpful to improve our workflow.

1. Added in b46f2d4
2. About the policy, possibly this requires more detail. Our goal is as described in the first issue comment: we want to keep a homogeneous package environment when appropriate.

For this, I see two review stages:

1. a PR review - Like Late task tutorials  #34 this review can be done in a feature dev branch. The aim of using sandpaper renv-family of functions initialize {renv} following sandpaper functions #16 is to facilitate reproducibility between developers and reviewers in this PR review. In this first stage, reviews will be capable to pull the feature dev branch and build the website locally, following the steps for a Reviewer contribution role.
2. a Website review - This aims at reviewers who need to review a website directly. This website is generated by the content in the main branch. An example of this is a previous tutorial we deployed (link). In this second stage review, reviewers can follow the How to contribute steps.

Now, I think in a scenario related to your question, and if not, please, I will need you to rephrase: What if the main and a dev branches have a different renv.lock file? Do we want to allow this? If yes, how to manage it? (we do not have a detailed policy for this, so happy to read team thoughts)

[pratikunterwegs]

Thanks Andree - an introduction on Nov 7th sounds good.

Now, I think in a scenario related to your question, and if not, please, I will need you to rephrase: What if the main and a dev branches have a different renv.lock file? Do we want to allow this? If yes, how to manage it? (we do not have a detailed policy for this, so happy to read team thoughts)

It will be necessary to add new packages to the lockfile on main from feature branches as new episodes cover new packages - such as adding {epidemics} in #34 - do you have a policy on the release status of these packages? Should they be on CRAN, or available as GH releases?

[avallecam]

do you have a policy on the release status of these packages? Should they be on CRAN, or available as GH releases?

We will need to keep writing new episodes independent of the package release status. So, initially, we want to tolerate available GH releases, registering specific versions if needed with pin_version. (the workbench runs actions weekly to keep the cache up to date - I still do not know what this can potentially change, like the latest SHA of a GH release?)

Now, if a GH release package is generating the job failing in #34 and will affect its merging, then this will condition us to wait for releases on CRAN. So, we can evaluate this policy after this first PR merging trial.

[avallecam]

From the r-universe PR#12 and its action failure message, Can I understand that one policy on the release status of package for a successful PR merge is that, if not on CRAN, it must fulfil two conditions (1) have a GH release and (2) be available in the r-universe?

[avallecam]

From the r-universe PR#12 and its action failure message, Can I understand that one policy on the release status of package for a successful PR merge is that, if not on CRAN, it must fulfil two conditions (1) have a GH release and (2) be available in the r-universe?

Rephrasing: One policy on the release status of packages for a successful PR merge is that, if not on CRAN, it must fulfil two conditions (1) be available on GH, and (2) be available in the r-universe.

(From the cited statement I removed the requirement of a "GH release", which I understand is not required, Can you help me confirm this @Bisaloo )

[avallecam]

1. I would only add that it's the developers' responsibility to keep their branches up to date with main, as with Update sandpaper cache, update digest #31 once it's merged.

@pratikunterwegs , can you help me confirm if these takeaways from the experience at #34 are accurate?

• Maintainer is responsible for keeping branches up to date with main.
  • Maintainer can follow the action of "update with rebase merge" in a PR to import last commits from main master.
  • However, the Maintainer should avoid doing multiple “merge branch 'main' into dev branch” during a PR. (this happens when "update with merge commits" is used, not recommended)
  • This should only be essential if the branch is having issues to compile or render, like requested in a comment of PR 34
  • Although there is a button to do it, this is not a suggested action to do at any time the main is updated.
  • The goal is to keep the history as linear as possible, as suggested in the blueprints.
• In a PR review, the Maintainer and Developer can wait until the last review to be done to do the “Rebase and merge” at the end only.

[pratikunterwegs]

I would say you should try to keep feature branches updated with main as much as possible.

An exception might be if the PR is scheduled to be merged after multiple others - in that case it might be easier to update just before merging, or before asking for a review.

In general it's better to ask for reviews on stable features or fixes, as this reduces the work for reviewers in keeping track of changes.

Maintainer can follow the action of "update with merge" in a PR to import last commits from master.

No, you should "update with rebase". Advance warning: you may or may not be able to do this online due to merge conflicts. If you have conflicts, you'll need to do this manually offline, and force-push the branch. If it does work online, your local branch will have a divergence with the remote, and you'll need to pull locally with a rebase.

[avallecam]

Maintainer can follow the action of "update with merge" in a PR to import last commits from master.

No, you should "update with rebase".

Yes! Update with rebase, thank you for catching that mistake.

And now I think how I made the mistake! I was pushing the "Update branch" button, assuming that I had checked with the "Update with rebase", but the default is "Update with merge commit". I knew clearly about the rebasing, but I think the default choice beat me. Lesson learned!

[Bisaloo]

From the r-universe PR#12 and its action failure message, Can I understand that one policy on the release status of package for a successful PR merge is that, if not on CRAN, it must fulfil two conditions (1) have a GH release and (2) be available in the r-universe?

Rephrasing: One policy on the release status of packages for a successful PR merge is that, if not on CRAN, it must fulfil two conditions (1) be available on GH, and (2) be available in the r-universe.

(From the cited statement I removed the requirement of a "GH release", which I understand is not required, Can you help me confirm this @Bisaloo )

I'm not sure what you are asking precisely.

Our policy (but not a technical requirement) to add packages on r-universe is for them to have at least one release, either on CRAN or on GitHub. There is a GitHub Actions workflow enforcing this on PR in epiverse-trace/epiverse-trace.r-universe.dev.

But this is not related to anything regarding the workbench.

[avallecam]

Our policy (but not a technical requirement) to add packages on r-universe is for them to have at least one release, either on CRAN or on GitHub. There is a GitHub Actions workflow enforcing this on PR in epiverse-trace/epiverse-trace.r-universe.dev.

But this is not related to anything regarding the workbench.

Thank you for the rephrasing. With your comment and this other comment carpentries/actions#32 (comment) I understood that the action failure came from the action for the package, but not from the workbench requirements.

Thank you all for your feedback. CONTRIBUTING file is now updated with the edit suggestion detailed in the linked issues above.