Skip to content

Flox manifest cleanup#629

Merged
forstmeier merged 1 commit intomasterfrom
flox-manifest-cleanup
Sep 18, 2025
Merged

Flox manifest cleanup#629
forstmeier merged 1 commit intomasterfrom
flox-manifest-cleanup

Conversation

@forstmeier
Copy link
Copy Markdown
Collaborator

@forstmeier forstmeier commented Sep 18, 2025

Overview

Changes

  • minor cleanup

Comments

In order to get tarpaulin working we'd need to update some stuff with rustup and use that instead of Flox directly.

Summary by CodeRabbit

  • Chores

    • Updated development environment: added cargo-tarpaulin for coverage tooling.
    • Removed mise and Google Cloud SDK from the default toolset.
    • No impact on application functionality.
  • Style

    • Cleaned up configuration by removing inline comments for consistency.
  • Tests

    • Test coverage tooling enabled via cargo-tarpaulin (no runtime impact).

@forstmeier forstmeier added this to the Refactor milestone Sep 18, 2025
@forstmeier forstmeier self-assigned this Sep 18, 2025
Copilot AI review requested due to automatic review settings September 18, 2025 17:28
@forstmeier forstmeier added the bug label Sep 18, 2025
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 18, 2025

Walkthrough

Updated .flox/env/manifest.toml [install] entries: removed mise and google-cloud-sdk, added cargo-tarpaulin, and cleaned inline comments on openssl and pkgconf values. No changes to scope or exported entities.

Changes

Cohort / File(s) Summary of Changes
Flox manifest install entries
\.flox/env/manifest.toml
Removed mise.pkg-path and google-cloud-sdk.pkg-path; added cargo-tarpaulin.pkg-path; normalized openssl.pkg-path and pkgconf.pkg-path by removing inline comments; no other logic changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5–10 minutes

Possibly related PRs

Suggested labels

rust

Suggested reviewers

  • chrisaddy

Poem

I hop through toml fields at night,
Pluck tarpaulin stars in gentle light—
Misе and clouds now out of sight,
Openssl’s path trimmed just right.
Thump-thump logs, my work is tight;
Carrots compiled, configs polite. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "Flox manifest cleanup" is concise, a single short phrase, and accurately reflects the primary change (cleanup of .flox/env/manifest.toml including package path updates and comment cleanup), so it clearly communicates the PR's main intent to reviewers.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch flox-manifest-cleanup

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR performs minor cleanup of the Flox manifest configuration file, removing unused packages and adding cargo-tarpaulin for test coverage analysis.

  • Removed unused packages (mise, google-cloud-sdk) from the development environment
  • Added cargo-tarpaulin for Rust test coverage functionality
  • Cleaned up inline comments for openssl and pkgconf packages

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@forstmeier forstmeier added tooling and removed bug labels Sep 18, 2025
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: 0

🧹 Nitpick comments (1)
.flox/env/manifest.toml (1)

18-18: Gate cargo-tarpaulin to Linux-only to avoid resolver/build failures on macOS.

cargo-tarpaulin is effectively Linux-only; leaving it unscoped will try to realize it on darwin systems in this env (your [options] includes darwin), which can break local dev shells and CI jobs on macOS.

Apply this diff:

-cargo-tarpaulin.pkg-path = "cargo-tarpaulin"
+cargo-tarpaulin.pkg-path = "cargo-tarpaulin"
+cargo-tarpaulin.systems = ["aarch64-linux", "x86_64-linux"]

Please confirm your CI matrix only installs/runs tarpaulin on Linux. If helpful, I can propose a follow-up that gates coverage steps by OS in your workflow.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a530ea and 1294c23.

⛔ Files ignored due to path filters (1)
  • .flox/env/manifest.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • .flox/env/manifest.toml (2 hunks)
⏰ 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). (8)
  • GitHub Check: Run Rust quality checks
  • GitHub Check: Run Python tests
  • GitHub Check: Run Rust tests
  • GitHub Check: Run Python quality checks
  • GitHub Check: Run Python quality checks
  • GitHub Check: Run Rust quality checks
  • GitHub Check: Run Python tests
  • GitHub Check: Run Rust tests
🔇 Additional comments (1)
.flox/env/manifest.toml (1)

38-39: Cleanup LGTM; consider OpenSSL version explicitness.

Removing inline comments is good. If the project requires a specific OpenSSL major (e.g., 3.x vs 1.1), consider pinning via the package name (e.g., openssl_3) to avoid accidental upgrades.

Does your Rust build (openssl-sys) work against the default OpenSSL on both Linux and macOS in this env? If not, I can suggest a minimal tweak (pkg-path pin or OPENSSL_DIR/PKG_CONFIG_* env in [hook].)

@forstmeier forstmeier merged commit 4442a89 into master Sep 18, 2025
3 checks passed
@forstmeier forstmeier deleted the flox-manifest-cleanup branch September 18, 2025 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants