Skip to content

fix(pacquet/config): satisfy dylint perfectionist lints#11672

Merged
zkochan merged 1 commit into
mainfrom
fix/dylint-trailing-comma
May 15, 2026
Merged

fix(pacquet/config): satisfy dylint perfectionist lints#11672
zkochan merged 1 commit into
mainfrom
fix/dylint-trailing-comma

Conversation

@zkochan

@zkochan zkochan commented May 15, 2026

Copy link
Copy Markdown
Member

Summary

#11526 landed two changes that trip the perfectionist Dylint lints under CI's RUSTFLAGS=-D warnings:

  1. perfectionist::macro-trailing-commacargo fmt collapsed a multi-line assert_eq!(...) into a single line in pacquet/crates/config/src/env_replace.rs:322 but kept the trailing comma. Removed the trailing comma.
  2. perfectionist::unicode_ellipsis_in_comments — a U+2026 in a test comment in pacquet/crates/config/src/npmrc_auth/tests.rs:146. Replaced with ASCII ....

Verified locally with cargo dylint --all -- --all-targets --workspace under RUSTFLAGS=-D warnings; passes clean. cargo fmt --check + cargo nextest run -p pacquet-config also clean.

Restores green Dylint on main: https://github.com/pnpm/pnpm/actions/runs/25923143087/job/76197188840

Test plan

  • RUSTFLAGS=-D warnings cargo dylint --all -- --all-targets --workspace — clean
  • cargo fmt --check -p pacquet-config — clean
  • cargo nextest run -p pacquet-config — all 154 tests pass

Written by an agent (Claude Code, claude-opus-4-7).

Summary by CodeRabbit

  • Tests
    • Fixed formatting in test assertions to remove erroneous artifacts.
    • Updated test comments for consistency.

Review Change Stack

Two pre-existing perfectionist findings landed via #11526 and broke
the Dylint CI job on main:

- `perfectionist::macro-trailing-comma` flagged a trailing comma in a
  single-line `assert_eq!(...)` that `cargo fmt` had collapsed from a
  multi-line form without dropping the comma. Removed the comma.
- `perfectionist::unicode_ellipsis_in_comments` warned about a U+2026
  `…` character in a test comment. CI runs with `RUSTFLAGS=-D warnings`,
  so the warning fails the build. Replaced with ASCII `...`.

Verified locally with `cargo dylint --all -- --all-targets --workspace`
under `RUSTFLAGS=-D warnings`; passes clean.

---
Written by an agent (Claude Code, claude-opus-4-7).
Copilot AI review requested due to automatic review settings May 15, 2026 15:04
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fff3a999-6deb-4e47-b35a-5a92f3a22184

📥 Commits

Reviewing files that changed from the base of the PR and between a62f959 and 5ab2cfa.

📒 Files selected for processing (2)
  • pacquet/crates/config/src/env_replace.rs
  • pacquet/crates/config/src/npmrc_auth/tests.rs
📜 Recent review details
⏰ 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). (11)
  • GitHub Check: Agent
  • GitHub Check: Format
  • GitHub Check: Lint and Test (windows-latest)
  • GitHub Check: Dylint
  • GitHub Check: Lint and Test (ubuntu-latest)
  • GitHub Check: Lint and Test (macos-latest)
  • GitHub Check: Code Coverage
  • GitHub Check: Analyze (javascript)
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Run benchmark on ubuntu-latest
  • GitHub Check: Compile & Lint
🧰 Additional context used
📓 Path-based instructions (1)
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

No star imports inside module bodies — write use super::{Foo, bar} instead of use super::*; and the same for any other glob. Exception: external-crate preludes like use rayon::prelude::*; and root-of-module re-exports like pub use submodule::*; in lib.rs

Files:

  • pacquet/crates/config/src/env_replace.rs
  • pacquet/crates/config/src/npmrc_auth/tests.rs
🔇 Additional comments (2)
pacquet/crates/config/src/env_replace.rs (1)

322-322: LGTM!

pacquet/crates/config/src/npmrc_auth/tests.rs (1)

146-146: LGTM!


📝 Walkthrough

Walkthrough

Two test files in the config crate receive cosmetic improvements: a trailing comma is removed from an assertion in env_replace.rs, and a typographic ellipsis in a comment is normalized to ASCII text in npmrc_auth/tests.rs. No test logic or behavior changes.

Changes

Test Code Formatting Fixes

Layer / File(s) Summary
Test assertion formatting and comment updates
pacquet/crates/config/src/env_replace.rs, pacquet/crates/config/src/npmrc_auth/tests.rs
Test assertion syntax is corrected by removing an extraneous trailing comma, and a comment ellipsis is updated to ASCII-style text. Both changes are cosmetic and do not affect test expectations or behavior.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A trailing comma hops away,
Comments cleaned in ASCII way,
Tests still pass, all tidy bright,
Formatting fixed—now all is right! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing Dylint perfectionist lints in the pacquet/config crate by removing trailing commas and fixing unicode characters in comments.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dylint-trailing-comma

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.12%. Comparing base (a62f959) to head (5ab2cfa).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11672      +/-   ##
==========================================
- Coverage   89.12%   89.12%   -0.01%     
==========================================
  Files         127      127              
  Lines       14470    14470              
==========================================
- Hits        12897    12896       -1     
- Misses       1573     1574       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

Copy link
Copy Markdown
Contributor

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.04      9.0±0.42ms   482.2 KB/sec    1.00      8.7±0.40ms   501.4 KB/sec

@github-actions

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.522 ± 0.080 2.397 2.679 1.00
pacquet@main 2.543 ± 0.073 2.378 2.622 1.01 ± 0.04
pnpm 4.884 ± 0.074 4.778 5.038 1.94 ± 0.07
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.5218007002999996,
      "stddev": 0.08048458994004838,
      "median": 2.5107317428,
      "user": 2.65282628,
      "system": 3.80478646,
      "min": 2.3968470438,
      "max": 2.6792207038,
      "times": [
        2.6792207038,
        2.4744011198,
        2.3968470438,
        2.5945165238,
        2.5798175268,
        2.4799992178,
        2.5011873427999998,
        2.5376190818,
        2.5202761428,
        2.4541222998
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.5428405004999997,
      "stddev": 0.07276099563299888,
      "median": 2.5439759982999997,
      "user": 2.66322748,
      "system": 3.81561296,
      "min": 2.3783311497999997,
      "max": 2.6215491807999998,
      "times": [
        2.5177548688,
        2.5912833408,
        2.5404486278,
        2.6057144867999997,
        2.6036740768,
        2.3783311497999997,
        2.5464685998,
        2.5414833968,
        2.4816972768,
        2.6215491807999998
      ]
    },
    {
      "command": "pnpm",
      "mean": 4.8841880367,
      "stddev": 0.07386265077030833,
      "median": 4.8609530563000005,
      "user": 8.33506648,
      "system": 4.1570728599999995,
      "min": 4.7777812898,
      "max": 5.038201545800001,
      "times": [
        5.038201545800001,
        4.8508719258,
        4.863881681800001,
        4.8540080668000005,
        4.929538706800001,
        4.8755278308,
        4.9621371938,
        4.8580244308000005,
        4.8319076948,
        4.7777812898
      ]
    }
  ]
}

Scenario: Frozen Lockfile (Hot Cache)

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 709.6 ± 32.0 683.8 794.1 1.00
pacquet@main 810.7 ± 74.2 739.1 947.3 1.14 ± 0.12
pnpm 2565.4 ± 147.9 2405.6 2897.5 3.62 ± 0.26
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.7096376627200002,
      "stddev": 0.0319685478184504,
      "median": 0.6986464853200001,
      "user": 0.37415040000000005,
      "system": 1.6075652200000001,
      "min": 0.6838100503200001,
      "max": 0.7940659753200001,
      "times": [
        0.7940659753200001,
        0.7181021133200001,
        0.6895228253200001,
        0.6838100503200001,
        0.6977144283200001,
        0.69203278032,
        0.6938637453200001,
        0.7071735853200001,
        0.69957854232,
        0.7205125813200001
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.8107224655200002,
      "stddev": 0.0742106506174786,
      "median": 0.78479027032,
      "user": 0.3752257,
      "system": 1.6327486199999999,
      "min": 0.7390579893200001,
      "max": 0.9473157643200001,
      "times": [
        0.9416297573200001,
        0.7773934103200001,
        0.76480541032,
        0.7544936603200001,
        0.9473157643200001,
        0.7390579893200001,
        0.7921871303200001,
        0.7678874493200001,
        0.8173488733200001,
        0.80510521032
      ]
    },
    {
      "command": "pnpm",
      "mean": 2.5653762981199995,
      "stddev": 0.1478990873379593,
      "median": 2.5083443893199995,
      "user": 3.2144634000000005,
      "system": 2.18858532,
      "min": 2.4056151623199997,
      "max": 2.89753138632,
      "times": [
        2.89753138632,
        2.74048660732,
        2.4927845713199996,
        2.55020463332,
        2.4607747403199998,
        2.5239042073199998,
        2.4056151623199997,
        2.49060754332,
        2.5999869683199996,
        2.49186716132
      ]
    }
  ]
}

@zkochan zkochan merged commit f05845a into main May 15, 2026
30 of 32 checks passed
@zkochan zkochan deleted the fix/dylint-trailing-comma branch May 15, 2026 18:59
github-actions Bot pushed a commit to Eyalm321/pnpm that referenced this pull request May 18, 2026
Two pre-existing perfectionist findings landed via pnpm#11526 and broke
the Dylint CI job on main:

- `perfectionist::macro-trailing-comma` flagged a trailing comma in a
  single-line `assert_eq!(...)` that `cargo fmt` had collapsed from a
  multi-line form without dropping the comma. Removed the comma.
- `perfectionist::unicode_ellipsis_in_comments` warned about a U+2026
  `…` character in a test comment. CI runs with `RUSTFLAGS=-D warnings`,
  so the warning fails the build. Replaced with ASCII `...`.

Verified locally with `cargo dylint --all -- --all-targets --workspace`
under `RUSTFLAGS=-D warnings`; passes clean.

---
Written by an agent (Claude Code, claude-opus-4-7).
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.

3 participants