Skip to content

Style improvements#73

Merged
m-jahn merged 5 commits intosnakemake:mainfrom
m-jahn:style_improvements
Mar 24, 2026
Merged

Style improvements#73
m-jahn merged 5 commits intosnakemake:mainfrom
m-jahn:style_improvements

Conversation

@m-jahn
Copy link
Copy Markdown
Member

@m-jahn m-jahn commented Mar 20, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added per-workflow documentation pages with visual diagrams, configuration and parameter details
  • Documentation

    • Switched local development and contribution instructions to use pixi and predefined test/render commands
    • Shortened commit/PR guidance
  • Bug Fixes

    • Faster, more reliable repository scanning with Snakefile-aware filtering and reduced delays
    • Improved workflow diagram styling
  • Chores

    • Tightened and normalized dependency version constraints

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e4f83ccd-f689-44a8-8d77-f018fe7f868a

📥 Commits

Reviewing files that changed from the base of the PR and between e43eba1 and c48f343.

📒 Files selected for processing (1)
  • scripts/common.py

📝 Walkthrough
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Style improvements' is vague and does not clearly convey the main objectives of the PR, which involve pixi migration, Snakefile filtering, and tube map generation. Consider a more descriptive title that captures the primary changes, such as 'Replace conda with pixi and add Snakefile filtering' or 'Improve workflow catalog generation with pixi, Snakefile filtering, and style enhancements'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed All code changes directly address the three linked issues: pixi migration replaces conda [#70], Snakefile detection filters repos [#71], and styling improvements support tube maps [#72].
Out of Scope Changes check ✅ Passed All changes are directly related to linked issues: dependency updates enable pixi, Snakefile filtering implements #71, and SVG styling supports tube maps from #72.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.

Add a .trivyignore file to your project to customize which findings Trivy reports.

@m-jahn m-jahn marked this pull request as ready for review March 20, 2026 16:47
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
scripts/generate-catalog.py (1)

142-142: Lambda captures loop variable by reference.

The lambda lambda: repo_search[i] captures i by reference. While this works correctly here because the lambda is called immediately within the same iteration, it's a common source of bugs if code is refactored.

Bind `i` explicitly
-        checked_repo = call_rate_limit_aware(lambda: repo_search[i], api_type="search")
+        checked_repo = call_rate_limit_aware(lambda idx=i: repo_search[idx], api_type="search")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/generate-catalog.py` at line 142, The lambda passed to
call_rate_limit_aware captures the loop variable i by reference (checked_repo =
call_rate_limit_aware(lambda: repo_search[i], ...)), which is fragile; fix by
binding the current index or value when creating the callable—for example, pass
a lambda that captures the value like lambda v=repo_search[i]: v or use
functools.partial to pass repo_search[i] into call_rate_limit_aware—so that
call_rate_limit_aware receives a callable that closes over the concrete
repo_search element rather than the loop variable i.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/common.py`:
- Around line 60-67: The has_snakefile function calls repo.get_contents directly
(for "Snakefile" and "workflow/Snakefile") and only catches
UnknownObjectException, which can leak other GithubException subclasses and
bypass rate-limit accounting; update has_snakefile to invoke repo.get_contents
inside call_rate_limit_aware (or wrap the get_contents call with the existing
rate-limit helper) and broaden the except clause to catch GithubException (or
Exception subclass appropriate) in addition to UnknownObjectException so
network/server errors are handled without crashing; keep the original return
semantics (True when found, False otherwise) and ensure any logged errors use
the existing logging pattern.

---

Nitpick comments:
In `@scripts/generate-catalog.py`:
- Line 142: The lambda passed to call_rate_limit_aware captures the loop
variable i by reference (checked_repo = call_rate_limit_aware(lambda:
repo_search[i], ...)), which is fragile; fix by binding the current index or
value when creating the callable—for example, pass a lambda that captures the
value like lambda v=repo_search[i]: v or use functools.partial to pass
repo_search[i] into call_rate_limit_aware—so that call_rate_limit_aware receives
a callable that closes over the concrete repo_search element rather than the
loop variable i.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 91038cfd-6836-423d-bdba-94bbebb7759e

📥 Commits

Reviewing files that changed from the base of the PR and between 30d9042 and e43eba1.

⛔ Files ignored due to path filters (1)
  • pixi.lock is excluded by !**/*.lock
📒 Files selected for processing (6)
  • README.md
  • pixi.toml
  • scripts/common.py
  • scripts/generate-catalog.py
  • source/build_wf_pages.py
  • source/modify_svg.py
💤 Files with no reviewable changes (1)
  • source/modify_svg.py

@m-jahn m-jahn merged commit a7697a8 into snakemake:main Mar 24, 2026
3 checks passed
@m-jahn m-jahn deleted the style_improvements branch March 26, 2026 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants