docs: Expand documentation#29
Merged
johanneskoester merged 13 commits intosnakemake:mainfrom Mar 12, 2026
Merged
Conversation
The function name by itself is a bit ambiguous.
To make the plugin section there more prominent.
Previously it was not possible to specify different packages due to changes in how Pixi handles env variable precedence.
Contributor
📝 WalkthroughWalkthroughAdds documentation and tooling for building plugin docs: README expanded with development/testing guidance, a new Pixi task Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR improves discoverability and contributor guidance for the Snakemake plugin catalog by adding clarifying docstrings, expanding landing-page documentation, and refining the Pixi task used to build docs for a subset of plugins in CI.
Changes:
- Added short docstrings to key plugin collection/metadata extraction helpers.
- Replaced the old single-plugin build task with a renamed
build-specifictask that supports multiple packages (and includes slurm by default for CI coverage). - Expanded the README and index template with links to upstream Snakemake plugin/architecture documentation and contribution/testing guidance.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
source/collect_plugins.py |
Adds docstrings to clarify plugin metadata collection and selection logic. |
source/_templates/index.rst.j2 |
Adds a link to upstream Snakemake architecture/plugin documentation for better discoverability. |
pixi.toml |
Introduces build-specific Pixi task with configurable plugin package list for targeted builds. |
README.md |
Adds contributing/testing guidance and links to plugin catalog + Snakemake docs. |
.github/workflows/ci.yml |
Updates CI to use the renamed build-specific task. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Task is too complex to be left inline.
More descriptive, it's possible to build multiple plugin docs with this task.
9a5eb91 to
23d998d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
31-34: Add one concretebuild-specificexample.This section names the task, but contributors still have to inspect
pixi.tomlto infer thepackagesargument format. A one-line example would make the workflow easier to discover.Suggested wording
### Testing Currently there are no unit-tests. Checking whether the code works as expected -is done by building individual plugin docs via the `build-specific` Pixi task. +is done by building individual plugin docs via the `build-specific` Pixi task, +for example: + +`pixi run build-specific --packages snakemake-executor-plugin-slurm`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 31 - 34, Add a one-line concrete example showing how to invoke the build-specific Pixi task and the expected packages argument format so contributors don't need to open pixi.toml; update the "Testing" section to include a sample command like using the build-specific task name "build-specific" and demonstrate the packages argument format (e.g., comma-separated package names or a single package) and reference pixi.toml for advanced options.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 6-7: The markdown link target is host-relative due to the leading
slash in "/source/_templates/index.rst.j2"; update the link in README.md to a
repository-relative path by removing the leading slash (change
"/source/_templates/index.rst.j2" to "source/_templates/index.rst.j2") so GitHub
resolves the template link correctly.
---
Nitpick comments:
In `@README.md`:
- Around line 31-34: Add a one-line concrete example showing how to invoke the
build-specific Pixi task and the expected packages argument format so
contributors don't need to open pixi.toml; update the "Testing" section to
include a sample command like using the build-specific task name
"build-specific" and demonstrate the packages argument format (e.g.,
comma-separated package names or a single package) and reference pixi.toml for
advanced options.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 400a41ea-0e1a-4140-afe4-823162f56b3d
📒 Files selected for processing (3)
.github/workflows/ci.ymlREADME.mdpixi.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
The single cluster-generic plugin does not contain additional docs, which the slurm plugin does. Adding the latter effectively increases test coverage.
23d998d to
a321a6c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
6-7:⚠️ Potential issue | 🟡 MinorUse a repository-relative link for the template.
Line 7 still points to
/source/_templates/index.rst.j2, which GitHub resolves from the site root instead of this repository. Dropping the leading slash fixes the README link.Suggested fix
-[this template](/source/_templates/index.rst.j2) +[this template](source/_templates/index.rst.j2)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 6 - 7, The README contains a repository-rooted link "/source/_templates/index.rst.j2" which GitHub resolves incorrectly; update the link target to the repository-relative path "source/_templates/index.rst.j2" (remove the leading slash) so the reference in the line containing the template link points to the correct file within this repo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pixi.toml`:
- Around line 14-20: The environment parsing for TEST_PACKAGES in
collect_plugins.py currently does a raw split(",") which leaves leading/trailing
whitespace on entries and causes exact-name comparisons later to fail; update
the TEST_PACKAGES parsing to trim each package (e.g., use a comprehension that
calls pkg.strip() and filters out empty strings) so comparisons in the
package-matching logic use normalized names without surrounding whitespace.
---
Duplicate comments:
In `@README.md`:
- Around line 6-7: The README contains a repository-rooted link
"/source/_templates/index.rst.j2" which GitHub resolves incorrectly; update the
link target to the repository-relative path "source/_templates/index.rst.j2"
(remove the leading slash) so the reference in the line containing the template
link points to the correct file within this repo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: af762cc6-69c3-423f-a163-17ae43a3f4f5
📒 Files selected for processing (2)
README.mdpixi.toml
As suggested by CodeRabbit (<snakemake#29 (comment)>).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@source/collect_plugins.py`:
- Around line 22-29: The current TEST_PACKAGES initialization treats an exported
empty string as set (producing []), so change the presence check to require a
non-empty value: use os.environ.get("TEST_PACKAGES") and ensure it's truthy
(e.g., value and value.strip()) before splitting; update the TEST_PACKAGES
assignment (the list comprehension that constructs TEST_PACKAGES) so that when
TEST_PACKAGES is an empty or whitespace-only string it falls back to None
instead of returning an empty list, preserving the default full collection
behavior used later (where TEST_PACKAGES is consulted).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: faaec72a-caed-4685-840f-93bd90974c26
📒 Files selected for processing (1)
source/collect_plugins.py
johanneskoester
approved these changes
Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I have realized as part of snakemake/snakemake#4052 that there is sufficient docs on documenting plugins, it was just not easy to discover. That PR, along with this, should close #27.
Summary by CodeRabbit
Documentation
Chores