-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
full review post-trial 01 and new episodes in tutorials-early #70
Conversation
Thank you!Thank you for your pull request 😃 🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}. If you have files that automatically render output (e.g. R Markdown), then you should check for the following:
Rendered Changes🔍 Inspect the changes: https://github.com/epiverse-trace/tutorials-early/compare/md-outputs..md-outputs-PR-70 The following changes were observed in the rendered markdown documents:
What does this mean?If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible. This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation. ⏱️ Updated at 2024-06-17 20:48:14 +0000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@avallecam, I have a few comments on this PR, please review the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Degoot-AM I finally managed to go through all the first four episodes! thank you work on them!
I prioritized code flow, that everything is running with the recent package versions. I found some edits required for cleanepi code, and one that I reported as issue.
Then I suggested to add some space between code lines within long chunks.
I added namespace reminders and some callout box of prerequisites as reminders of the dataset that we need to download before going through the episode.
For the first episode on read case data, we may open it to four subtitles to add brief details on each step. Also I suggested a figure as a first intro to databases.
Regarding the setup page, better if we focus to update the download links and the packages required for the four episodes. We had mixed and overlapping suggestions here and in the PR #64 so better if we reconciliate them after merging these most recent edits.
Are you able to approve commits in this PR? If yes, please, go ahead!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @avallecam for you feedback.
.github/workflows/README.md
Outdated
@@ -96,7 +96,7 @@ are okay. | |||
|
|||
This update is run ~~weekly or~~ on demand. | |||
|
|||
### 03 Maintain: Update Pacakge Cache (update-cache.yaml) | |||
### 03 Maintain: Update Package Cache (update-cache.yaml) | |||
|
|||
For lessons that have generated content, we use {renv} to ensure that the output | |||
is stable. This is controlled by a single lockfile which documents the packages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is stable. This is controlled by a single lockfile which documents the packages | |
is stable. This is controlled by our (workflow) [https://github.com/epiverse-trace/tutorials-early/tree/main/.github/workflows] that can generate (PRs to update) [https://github.com/epiverse-trace/research-compendium/pull/38] package versions in the (lockfile) [https://github.com/epiverse-trace/tutorials-early/blob/main/renv/profiles/lesson-requirements/renv.lock]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this file is a workflow file from the workbench infrastructure, better to propose edits in the carpentries template file
redirecting this PR to main now |
Co-authored-by: Hugo Gruson <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Andree Valle Campos <[email protected]>
Co-authored-by: Andree Valle Campos <[email protected]>
Co-authored-by: Andree Valle Campos <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
Co-authored-by: Abdoelnaser M Degoot <[email protected]>
f0a8e6f
to
8fb5d66
Compare
This PR aims to collect edit suggestions for the tutorials-early repository.
This includes any specific addition to:
All rendered content is in https://epiverse-trace.github.io/tutorials-early/