Conversation
|
Warning Rate limit exceeded@cmeesters has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds cluster-specific configuration entries and a Bonn workshop config, provisioning scripts, Conda settings, new/modified slides (HPC job example, Conda guidance, editors, contributing workflows), slide reordering, GPU/resource schema updates, and logging/resource tweaks in tutorials. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Local as Local Host
participant SSH as SSH
participant Remote as Marvin/Remote
rect rgb(235,245,255)
Note over Local,Remote: setup_creators/copy_script_UniBonn.sh provisioning flow
end
Local->>SSH: ssh user@remote "mkdir -p $BASEPATH"
Local->>Remote: scp condarc_bonn get_tutorial.sh environment.yaml config.yaml
Local->>Remote: rsync -av --chmod=D755 tutorial/ $REMOTE/tutorial/
Local->>Remote: rsync -av --chmod=D755 solutions/ $REMOTE/solutions/
Note over Remote: files and profiles installed on remote host
sequenceDiagram
autonumber
participant User as Workshop attendee
participant Snakemake as Snakemake
participant Cluster as SLURM cluster
rect rgb(245,255,235)
Note over User,Cluster: Job submission using parameterized latency and storage prefixes
end
User->>Snakemake: snakemake --cluster "sbatch ..." --latency-wait=<<cluster.latency>>
Snakemake->>Cluster: submit job (uses remotejoblocalstorageprefix/localstorageprefix)
Cluster->>Snakemake: job queued / started
Snakemake->>User: job output written to configured `localstorageprefix`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Artifacts |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
slides/creators/Selecting_Workflows.tex (1)
126-139: Consider removing commented-out frame.The "Finalizing the Deployment" frame is entirely commented out. If this content is no longer needed for the course, consider removing it entirely rather than leaving it as commented code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
images/Snakemake/Snakemake_Workflow_Catalog.pngis excluded by!**/*.pngimages/contact/announcement_robot_example_screenshot.pngis excluded by!**/*.pngimages/misc/publish-to-earth.pngis excluded by!**/*.png
📒 Files selected for processing (23)
config/config.yaml(1 hunks)config/config_Bonn.yaml(1 hunks)config/config_Erlangen_Admin.yaml(1 hunks)config/config_Mainz_NHR.yaml(2 hunks)config/config_dresden.yaml(1 hunks)setup_creators/condarc_bonn(1 hunks)setup_creators/config.yaml(1 hunks)setup_creators/copy_script_UniBonn.sh(1 hunks)setup_creators/tutorial/profile/Bonn_Marvin/config.yaml(1 hunks)slides/Snakemake_HPC_User_Creator_Combi.tex(2 hunks)slides/common/About_Snakemake.tex(2 hunks)slides/common/Contributing.tex(2 hunks)slides/common/HPC_101.tex(0 hunks)slides/common/Hello_World_HPC_Marvin.tex(1 hunks)slides/common/InTouch.tex(2 hunks)slides/common/condarc_bonn.tex(1 hunks)slides/common/plain_editor_bonn.tex(1 hunks)slides/common/software_environment.tex(4 hunks)slides/creators/Decorating_the_Workflow.tex(2 hunks)slides/creators/Parallel_Execution.tex(1 hunks)slides/creators/Selecting_Workflows.tex(3 hunks)slides/creators/Workflow_Parameterization_for_HPC.tex(2 hunks)slides/creators/contributing_workflows.tex(1 hunks)
💤 Files with no reviewable changes (1)
- slides/common/HPC_101.tex
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/07_Snakefile_script:11-19
Timestamp: 2024-10-21T19:12:04.910Z
Learning: In the Snakemake HPC teaching material, parallel execution and threading are discussed at a later stage in the course, so suggestions related to adding a `threads` directive or parallel execution should be avoided in earlier sections.
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 178
File: slides/common/login_mogon.tex:304-306
Timestamp: 2025-01-17T13:32:29.634Z
Learning: In educational materials and presentation slides, code examples should be kept simple and easy to type from the screen. Complex safety checks and automation patterns should be avoided as they make it harder for participants to follow along during live demonstrations. This is especially important for teaching materials in the snakemake-hpc-teaching-material repository.
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: slides/creators/Python_in_Snakemake.tex:463-463
Timestamp: 2024-10-07T18:50:01.657Z
Learning: In the project, placeholders like '<++cluster.display_program++>' in `slides/creators/Python_in_Snakemake.tex` are intentionally used and will be replaced by values from a configuration file using Jinja2. Do not flag these placeholders as needing replacement.
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: slides/creators/Python_in_Snakemake.tex:463-463
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In the project, placeholders like '<++cluster.display_program++>' in `slides/creators/Python_in_Snakemake.tex` are intentionally used and will be replaced by values from a configuration file using Jinja2. Do not flag these placeholders as needing replacement.
📚 Learning: 2024-10-21T18:54:07.179Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: slides/creators/Decorating_the_Workflow.tex:165-166
Timestamp: 2024-10-21T18:54:07.179Z
Learning: In the `slides/creators/Decorating_the_Workflow.tex`, placeholder paths like `\altverb{<++course.pathtosolutions++>/...}` are intentionally left unchanged and should not be replaced with actual paths.
Applied to files:
slides/creators/Decorating_the_Workflow.texslides/creators/Selecting_Workflows.texconfig/config_Bonn.yamlslides/common/plain_editor_bonn.texslides/creators/Workflow_Parameterization_for_HPC.texslides/Snakemake_HPC_User_Creator_Combi.texconfig/config_Mainz_NHR.yamlslides/creators/contributing_workflows.tex
📚 Learning: 2025-01-25T20:24:32.960Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 180
File: config/config_Erlangen_Admin.yaml:0-0
Timestamp: 2025-01-25T20:24:32.960Z
Learning: The YAML configuration files in the snakemake-hpc-teaching-material repository, particularly under config/, are used directly without variable expansion/interpolation support. Therefore, paths must be specified as literal values without using variables like ${base_path}.
Applied to files:
slides/creators/Decorating_the_Workflow.texconfig/config_Bonn.yamlslides/creators/Workflow_Parameterization_for_HPC.tex
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: slides/creators/Python_in_Snakemake.tex:463-463
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In the project, placeholders like '<++cluster.display_program++>' in `slides/creators/Python_in_Snakemake.tex` are intentionally used and will be replaced by values from a configuration file using Jinja2. Do not flag these placeholders as needing replacement.
Applied to files:
slides/creators/Decorating_the_Workflow.texslides/creators/Selecting_Workflows.texconfig/config_Bonn.yamlslides/creators/Workflow_Parameterization_for_HPC.texslides/Snakemake_HPC_User_Creator_Combi.texslides/creators/contributing_workflows.texconfig/config.yamlslides/common/software_environment.tex
📚 Learning: 2024-10-21T18:47:10.172Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/07_Snakefile_script:40-50
Timestamp: 2024-10-21T18:47:10.172Z
Learning: In the Snakemake demo workflows under `setup_creators/solutions`, such as `07_Snakefile_script`, parallel execution and threading are intentionally omitted at this stage and should not be suggested.
Applied to files:
slides/creators/Decorating_the_Workflow.texslides/creators/Workflow_Parameterization_for_HPC.tex
📚 Learning: 2024-10-21T19:12:04.910Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/07_Snakefile_script:11-19
Timestamp: 2024-10-21T19:12:04.910Z
Learning: In the Snakemake HPC teaching material, parallel execution and threading are discussed at a later stage in the course, so suggestions related to adding a `threads` directive or parallel execution should be avoided in earlier sections.
Applied to files:
slides/creators/Decorating_the_Workflow.texslides/creators/Workflow_Parameterization_for_HPC.tex
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 157
File: slides/creators/A_First_Workflow.tex:124-136
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `slides/creators/A_First_Workflow.tex`, when presenting Snakefiles, prefer to use multiple single-quoted strings concatenated in the `shell` directive for better readability on the slides, instead of using triple-quoted strings.
Applied to files:
slides/creators/Selecting_Workflows.texslides/common/About_Snakemake.texslides/creators/Workflow_Parameterization_for_HPC.texslides/Snakemake_HPC_User_Creator_Combi.texslides/creators/contributing_workflows.texslides/common/software_environment.tex
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: slides/creators/Python_in_Snakemake.tex:262-265
Timestamp: 2024-10-08T17:41:54.542Z
Learning: Placeholders like '<++course.pathtosolutions++>' in 'slides/creators/Python_in_Snakemake.tex' are automatically rendered using Jinja2 and replaced with values from a configuration file during the build process.
Applied to files:
slides/creators/Selecting_Workflows.texconfig/config_Bonn.yamlslides/creators/Workflow_Parameterization_for_HPC.texslides/Snakemake_HPC_User_Creator_Combi.texconfig/config_Mainz_NHR.yamlslides/common/software_environment.tex
📚 Learning: 2024-10-21T19:07:46.414Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: slides/creators/Decorating_the_Workflow.tex:376-380
Timestamp: 2024-10-21T19:07:46.414Z
Learning: In the LaTeX slides (`slides/creators/Decorating_the_Workflow.tex`), the macros `\bcinfo`, `\bcattention`, and `\bcquestion` are custom-defined within the slide set as images and are intentionally used.
Applied to files:
slides/creators/Selecting_Workflows.texslides/common/plain_editor_bonn.texslides/creators/contributing_workflows.tex
📚 Learning: 2024-10-21T18:32:55.000Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/06_Snakefile_run:39-42
Timestamp: 2024-10-21T18:32:55.000Z
Learning: In `setup_creators/solutions/06_Snakefile_run`, the initial use of the software is temporary; later content will introduce a proper wrapper tool to handle it more effectively.
Applied to files:
slides/creators/Selecting_Workflows.texslides/common/software_environment.tex
📚 Learning: 2025-01-17T13:32:29.634Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 178
File: slides/common/login_mogon.tex:304-306
Timestamp: 2025-01-17T13:32:29.634Z
Learning: In educational materials and presentation slides, code examples should be kept simple and easy to type from the screen. Complex safety checks and automation patterns should be avoided as they make it harder for participants to follow along during live demonstrations. This is especially important for teaching materials in the snakemake-hpc-teaching-material repository.
Applied to files:
config/config_Bonn.yaml
📚 Learning: 2024-10-21T19:07:01.028Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: slides/creators/Decorating_the_Workflow.tex:379-380
Timestamp: 2024-10-21T19:07:01.028Z
Learning: In the LaTeX files of this project, the `\bc...` commands (such as `\bcattention`) are custom-defined and valid within the slide set. Warnings about them being undefined are unnecessary.
Applied to files:
slides/common/plain_editor_bonn.tex
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: setup_creators/solutions/06_Snakefile_run:61-63
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `setup_creators/solutions/06_Snakefile_run` (Python, Snakemake), during early-stage exercises where configuration options are intentionally not introduced yet, parameters like `window_size` may remain hardcoded. These will be made configurable in more advanced versions or future pull requests.
Applied to files:
slides/creators/Workflow_Parameterization_for_HPC.tex
📚 Learning: 2025-02-09T13:27:52.874Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 181
File: setup_creators/get_tutorial.sh:3-4
Timestamp: 2025-02-09T13:27:52.874Z
Learning: The commands in setup_creators/get_tutorial.sh are meant to be executed interactively by participants during tutorials/courses, not as an automated script. Therefore, they should be kept simple and readable without additional error handling.
Applied to files:
setup_creators/copy_script_UniBonn.sh
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: slides/creators/Python_in_Snakemake.tex:171-171
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `slides/creators/Python_in_Snakemake.tex`, the `@` symbols around code are used to indicate characters to be highlighted; they themselves will not be visible on the slides.
Applied to files:
slides/common/software_environment.tex
🔇 Additional comments (31)
config/config_dresden.yaml (1)
61-62: LGTM: Consistent latency configuration.The addition of the
latencyfield with a sensible default value of 30 seconds for NFS filesystems is appropriate and aligns with similar additions across other cluster configurations in this PR.slides/common/InTouch.tex (2)
47-47: LGTM: Helpful user-facing addition.Adding information about the Fediwall extension that doesn't require a Fediverse account improves accessibility for course participants.
58-58: LGTM: Link update for SLURM plugin.The explicit URL link for the SLURM plugin is correctly updated.
config/config.yaml (1)
43-57: LGTM: Complete cluster configuration template.The new cluster configuration block provides a comprehensive example for the Marvin cluster, including appropriate defaults for latency, storage prefixes, and other cluster-specific settings. This aligns with the template nature of this config file.
slides/creators/Parallel_Execution.tex (1)
325-346: LGTM: GPU support documentation updated.The frame title change from "Outlook" to current tense and the updated resource configuration examples reflect the maturation of GPU support in Snakemake. The new generalized schema with
single_gpu_rule,multi_gpu_rule, andgres_request_ruleprovides clearer guidance for different GPU allocation scenarios.config/config_Erlangen_Admin.yaml (1)
70-71: LGTM: Consistent latency configuration.The latency field addition matches the pattern applied across other cluster configurations in this PR and provides appropriate documentation for NFS filesystem considerations.
config/config_Mainz_NHR.yaml (2)
25-25: LGTM: Course date updated.The date change to "September 2025" appears intentional for course scheduling.
72-73: LGTM: Consistent latency configuration.The latency field addition aligns with the broader PR pattern of explicitly configuring filesystem latency across cluster environments.
setup_creators/condarc_bonn (1)
1-9: Other Conda settings look appropriate for a workshop environment.The channel configuration, automatic confirmation (
always_yes: true), and strict channel priority are reasonable choices for a teaching/workshop setup to minimize installation friction.slides/common/About_Snakemake.tex (2)
140-140: Based on the web search result, Snakemake provides a Nextflow wrapper (utils/nextflow) that lets you run Nextflow pipelines from Snakemake, and this support is marked as stable. The statement in line 140 about Snakemake's ability to carry out Nextflow wrappers is accurate and current. No clarification or changes are needed.
70-71: Unable to verify workflow counts through web search.The Snakemake Workflow Catalog's workflow counts are dynamically generated on the live website and not indexed by search engines. I cannot confirm whether "over 2000 workflows" and "Almost 400 standardized workflows" reflect the current catalog state as of November 2025.
To verify these figures, you should:
- Visit https://snakemake.github.io/snakemake-workflow-catalog/ directly to check the current count displayed
- Review the "All standardized workflows" and "All other workflows" pages to see actual numbers
The slide content is otherwise appropriate for a course document, but these statistics should be confirmed against the live catalog before use.
slides/common/Contributing.tex (2)
2-2: LGTM! Enhanced section title clarifies scope.The updated section title "Contributing -- Beyond Code" better communicates that contributions extend beyond programming, aligning well with the slide content about issue reporting, documentation, and community involvement.
37-40: LGTM! New subsection improves slide organization.The addition of the "Reporting Issues" subsection with appropriate spacing creates better structural organization for the presentation content that follows.
setup_creators/tutorial/profile/Bonn_Marvin/config.yaml (1)
11-12: LGTM! Default partition configured correctly.The default resource specification for the SLURM partition is properly configured for the Bonn Marvin cluster.
slides/creators/Decorating_the_Workflow.tex (2)
691-691: LGTM! Directory name updated consistently.The profile directory reference has been correctly updated from "profile" to "profiles" throughout the slide deck.
712-716: LGTM! Command examples updated with new directory name.The copy command and workflow-profile flag have been consistently updated to reference the "profiles" directory, maintaining coherence across the teaching materials.
slides/creators/Selecting_Workflows.tex (2)
95-95: LGTM! Updated to use conda instead of mamba.The environment creation command now uses
condainstead ofmamba, aligning with the broader shift in the teaching materials reflected in other slide updates.
115-115: LGTM! URL placeholder updated for clarity.The deployment URL placeholder changed from
<URL>to> some github url, providing a clearer example format for students.slides/common/Hello_World_HPC_Marvin.tex (1)
1-44: LGTM! Workshop-specific HPC job example.This slide provides a clear, simple first job script example for the Bonn workshop on the Marvin cluster. The hardcoded account, partition, and reservation values are appropriate for course-specific teaching materials.
Based on learnings: Educational materials should keep examples simple and easy to type from the screen.
slides/common/plain_editor_bonn.tex (1)
1-88: LGTM! Clear editor guidance for Bonn workshop.This slide provides practical, Bonn-specific guidance on available editors and IDEs for the Marvin cluster. The content is well-structured with clear examples suitable for teaching materials.
slides/common/condarc_bonn.tex (2)
5-18: Verify ssl_verify: false is necessary for Marvin cluster.The example
.condarcincludesssl_verify: false(line 14), which disables SSL certificate verification. While this may be necessary for HPC clusters with custom certificate authorities, confirm this setting is required for the Marvin cluster environment.If SSL verification can be enabled, it would improve security. Otherwise, consider adding a comment explaining why this setting is needed for the cluster.
1-25: LGTM! Helpful Conda configuration for HPC environment.This slide provides practical guidance for reducing Conda search overhead on HPC clusters through a pre-configured
.condarcfile. The channel priorities and settings are appropriate for bioinformatics workflows.setup_creators/config.yaml (1)
8-11: Verify empty prefix blocks are intentional.The
remote-job-local-prefixandlocal-storage-prefixblocks are empty. Confirm whether these should remain empty as placeholders or require default values for the Bonn workshop setup.config/config_Bonn.yaml (1)
1-79: LGTM! Well-structured Bonn workshop configuration.The configuration file follows the established pattern from other cluster configs in the repository. All paths are specified as literal values (as required), cluster parameters are consistent, and Jinja2 placeholders are properly used throughout the slide references.
slides/creators/Workflow_Parameterization_for_HPC.tex (2)
166-173: LGTM! Good parameterization of latency values.The replacement of hard-coded latency value with the
<++cluster.latency++>placeholder allows cluster-specific configuration. The added warning about NFS file systems and latency values provides helpful context for course participants.
185-187: LGTM! Storage prefix parameters enhance I/O configuration.The addition of
remote-job-local-storage-prefixandlocal-storage-prefixparameters complements the latency configuration and provides a complete I/O optimization setup for HPC workflows.Also applies to: 200-202
slides/common/software_environment.tex (3)
162-162: Good update on pixi support status.The change from "not yet supported" to "will be supported...soon" reflects updated information about pixi integration with Snakemake.
186-199: LGTM! Marvin-specific approach simplifies Conda setup.The new approach using
module load Miniforgefollowed byconda initis cleaner for the Bonn workshop and avoids the complexity of manual Conda installation, making it easier for participants to follow along. Based on learnings.
412-412: Consistent command updates for Conda.The changes from
mambatocondacommands align with the module-based Miniforge approach introduced for Marvin, ensuring consistency throughout the slide deck.Also applies to: 422-422, 430-430
slides/Snakemake_HPC_User_Creator_Combi.tex (2)
90-91: LGTM! Logical placement of HPC_101 content.Placing the HPC_101 introduction after the software environment setup and before workflow parameterization creates a logical progression for learners, ensuring they understand HPC basics before diving into cluster-specific configuration.
108-109: LGTM! Good addition of contributing workflows section.The placement of the contributing workflows section after modularization and before the general contributing section provides a natural flow for learners to understand how to publish and share their work.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/announce-release.yml (1)
17-35: Missing step definition:extract-release-notesstep is undefined.Line 35 references
${{ steps.extract-release-notes.outputs.release_notes }}, but the job has no step with IDextract-release-notesdefined. This will cause the workflow to fail at runtime when attempting to interpolate the expression.Add a step before the "Post to Mastodon" step to extract release notes from CHANGELOG.md. For example:
- name: Post to Mastodon + - name: Extract Release Notes + id: extract-release-notes + run: | + # Extract release notes from CHANGELOG.md + # This step should populate outputs.release_notes
🧹 Nitpick comments (3)
setup_creators/solutions/09_Snakefile_config (1)
1-1: Consider clarifying the comment.The comment "our samples are pre-configured" is somewhat vague. Since this is the config stage of the tutorial (file 09), consider making it more explicit that samples are loaded from the configuration file, e.g.,
# samples are loaded from config.yaml.Apply this diff to improve clarity:
-# our samples are pre-configured +# samples are loaded from config.yamlsetup_creators/tutorial/11_Snakefile_rule_params (1)
65-80: Consider adding log directives to script-based rules for consistency.While the shell-based rules now have proper log directives, the script-based rules (
plot_positionsandplot_quals) do not. Although scripts typically manage their own output, adding log directives would provide consistency across all rules and allow scripts to write errors or debug information to dedicated log files viasnakemake.log.This is optional and may depend on the pedagogical goals of this tutorial step.
Example for the
plot_positionsrule:rule plot_positions: input: rules.bcftools_call.output, output: "calls/positions.png", + log: + "logs/plot_positions/plot_positions.log", script: "scripts/plot-positions.py".github/workflows/announce-release.yml (1)
19-19: Action pinned to@maininstead of stable release.The action reference uses
@main(latest development version) rather than a stable release tag. While the comment indicates this is intentional, consider the operational risk: breaking changes or bugs in the latest development version could disrupt release announcements.If using
@mainis necessary, ensure your team monitors the upstream action repository for breaking changes. Alternatively, consider pinning to a specific stable release.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/announce-release.yml(1 hunks).github/workflows/release-please.yml(2 hunks)setup_creators/solutions/08_Snakefile_script2(0 hunks)setup_creators/solutions/09_Snakefile_config(1 hunks)setup_creators/tutorial/11_Snakefile_rule_params(1 hunks)slides/creators/Decorating_the_Workflow.tex(4 hunks)
💤 Files with no reviewable changes (1)
- setup_creators/solutions/08_Snakefile_script2
🚧 Files skipped from review as they are similar to previous changes (1)
- slides/creators/Decorating_the_Workflow.tex
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/07_Snakefile_script:11-19
Timestamp: 2024-10-21T19:12:04.910Z
Learning: In the Snakemake HPC teaching material, parallel execution and threading are discussed at a later stage in the course, so suggestions related to adding a `threads` directive or parallel execution should be avoided in earlier sections.
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 178
File: slides/common/login_mogon.tex:304-306
Timestamp: 2025-01-17T13:32:29.634Z
Learning: In educational materials and presentation slides, code examples should be kept simple and easy to type from the screen. Complex safety checks and automation patterns should be avoided as they make it harder for participants to follow along during live demonstrations. This is especially important for teaching materials in the snakemake-hpc-teaching-material repository.
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: slides/creators/Python_in_Snakemake.tex:463-463
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In the project, placeholders like '<++cluster.display_program++>' in `slides/creators/Python_in_Snakemake.tex` are intentionally used and will be replaced by values from a configuration file using Jinja2. Do not flag these placeholders as needing replacement.
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: slides/creators/Python_in_Snakemake.tex:463-463
Timestamp: 2024-10-07T18:50:01.657Z
Learning: In the project, placeholders like '<++cluster.display_program++>' in `slides/creators/Python_in_Snakemake.tex` are intentionally used and will be replaced by values from a configuration file using Jinja2. Do not flag these placeholders as needing replacement.
📚 Learning: 2025-02-17T19:51:43.514Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 0
File: :0-0
Timestamp: 2025-02-17T19:51:43.514Z
Learning: When creating GitHub releases, if artifacts are already built and uploaded in another workflow, use actions/download-artifact to access them instead of rebuilding, which saves time and ensures consistency.
Applied to files:
.github/workflows/release-please.yml
📚 Learning: 2024-11-02T19:43:26.339Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 169
File: .github/workflows/post_to_mastodon.yml:13-17
Timestamp: 2024-11-02T19:43:26.339Z
Learning: In the GitHub Actions workflow `Post to Mastodon on PR Merge` (file: `.github/workflows/post_to_mastodon.yml`), additional checks or error handling for the Mastodon integration are not required as per cmeesters's preference.
Applied to files:
.github/workflows/announce-release.yml
📚 Learning: 2024-10-21T18:47:45.192Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/07_Snakefile_script:67-70
Timestamp: 2024-10-21T18:47:45.192Z
Learning: In the `setup_creators/solutions/07_Snakefile_script`, the tutorial data ensures that the VCF files are not empty, so handling empty VCF files in the `plot_positions` rule is unnecessary.
Applied to files:
setup_creators/tutorial/11_Snakefile_rule_paramssetup_creators/solutions/09_Snakefile_config
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: setup_creators/solutions/06_Snakefile_run:49-83
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `setup_creators/solutions/06_Snakefile_run`, the code within the `run` block of the `plot_positions` rule is intentionally placed there to demonstrate the `run` directive and to assess readability.
Applied to files:
setup_creators/tutorial/11_Snakefile_rule_params
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: setup_creators/solutions/06_Snakefile_run:61-63
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In `setup_creators/solutions/06_Snakefile_run` (Python, Snakemake), during early-stage exercises where configuration options are intentionally not introduced yet, parameters like `window_size` may remain hardcoded. These will be made configurable in more advanced versions or future pull requests.
Applied to files:
setup_creators/tutorial/11_Snakefile_rule_paramssetup_creators/solutions/09_Snakefile_config
📚 Learning: 2024-10-21T18:48:51.348Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/tutorial/06_Snakefile_run:16-17
Timestamp: 2024-10-21T18:48:51.348Z
Learning: In `setup_creators/tutorial/06_Snakefile_run`, participants of the course are expected to fill in missing code themselves. Any incomplete code in this file is intentional and should not be modified.
Applied to files:
setup_creators/tutorial/11_Snakefile_rule_paramssetup_creators/solutions/09_Snakefile_config
📚 Learning: 2024-10-08T17:41:54.542Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: setup_creators/solutions/08_Snakefile_script2:2-2
Timestamp: 2024-10-08T17:41:54.542Z
Learning: In the `setup_creators/solutions/08_Snakefile_script2` file, samples are hardcoded with `SAMPLES = ["A", "B"]` because configurability will be explained at a later stage in the course.
Applied to files:
setup_creators/solutions/09_Snakefile_config
📚 Learning: 2024-10-21T18:46:35.006Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/06_Snakefile_run:65-75
Timestamp: 2024-10-21T18:46:35.006Z
Learning: In `setup_creators/solutions/06_Snakefile_run`, the tutorial data always contains variants, so handling empty variant lists in the code is unnecessary.
Applied to files:
setup_creators/solutions/09_Snakefile_config
📚 Learning: 2024-10-21T18:32:55.000Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/solutions/06_Snakefile_run:39-42
Timestamp: 2024-10-21T18:32:55.000Z
Learning: In `setup_creators/solutions/06_Snakefile_run`, the initial use of the software is temporary; later content will introduce a proper wrapper tool to handle it more effectively.
Applied to files:
setup_creators/solutions/09_Snakefile_config
📚 Learning: 2024-10-07T18:47:39.827Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 158
File: setup_creators/solutions/07_Snakefile_script:67-69
Timestamp: 2024-10-07T18:47:39.827Z
Learning: In `setup_creators/solutions/07_Snakefile_script`, it's acceptable not to handle empty variant lists since the sample data is guaranteed to produce variants.
Applied to files:
setup_creators/solutions/09_Snakefile_config
📚 Learning: 2024-10-21T18:50:29.046Z
Learnt from: cmeesters
Repo: snakemake/snakemake-hpc-teaching-material PR: 161
File: setup_creators/tutorial/06_Snakefile_run:85-92
Timestamp: 2024-10-21T18:50:29.046Z
Learning: In `setup_creators/tutorial/06_Snakefile_run`, the code is intentionally unordered as part of an exercise for learners to reorder according to conventions.
Applied to files:
setup_creators/solutions/09_Snakefile_config
🪛 actionlint (1.7.8)
.github/workflows/announce-release.yml
23-23: property "extract-release-notes" is not defined in object type {}
(expression)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (9)
setup_creators/solutions/09_Snakefile_config (2)
3-3: LGTM!The config file loading is correctly implemented for this stage of the tutorial.
43-44: LGTM! Config-driven samples correctly implemented.The transition from hardcoded samples to
config["samples"]is appropriate for this stage of the tutorial. Theexpand()calls correctly reference the configuration, which aligns with the pedagogical progression where configuration is introduced at this stage (file 09).setup_creators/tutorial/11_Snakefile_rule_params (4)
20-25: Excellent addition of log capture for the bwa_map pipeline.The log directive and stderr redirection are correctly implemented. Wrapping the entire pipeline in parentheses ensures that stderr from both
bwa memandsamtools viewis captured in the log file, which is the correct approach.
33-37: Good addition of log capture for samtools_sort.The log directive and stderr redirection are correctly implemented.
45-48: Good addition of log capture for samtools_index.The log directive and stderr redirection are correctly implemented.
58-62: Excellent addition of log capture for the bcftools_call pipeline.The log directive and stderr redirection are correctly implemented. The parentheses properly capture stderr from both
bcftools mpileupandbcftools call..github/workflows/release-please.yml (3)
46-50: No changes needed—both directories exist.Verification confirms that
setup_creators/solutions/andsetup_creators/tutorial/are present in the repository with content. The zip command will execute successfully without modifications.
35-44: Artifact download configuration verified as correct.The
build.ymlworkflow correctly produces and uploads aSlidesartifact (line 43, containingslides/*.pdffiles). The artifact name matches exactly whatrelease-please.ymlexpects, so the strict failure checks will work as intended.
52-61: No issues found. Code is correct.The v4 release-please-action provides
tag_nameas a root-component output, and the workflow correctly exposes it in line 19 and uses it in line 57. The action version (v4) and output mapping are both valid.
1 participant
Summary by CodeRabbit
New Features
Documentation
Bug Fixes