refactor(npmrc): collapse `WorkspaceSettings::apply_to` with macro by KSXGitHub · Pull Request #406 · pnpm/pacquet

KSXGitHub · 2026-05-08T00:52:10Z

Summary by CodeRabbit

Refactor
- Optimized internal workspace configuration processing for improved code maintainability.

Most field assignments in apply_to follow the same shape — `if let Some(v) = self.X { npmrc.X = v; }` — and the path-resolved fields share a second shape. Introduce a small function-local macro with two arms (direct assignment, base-dir resolution) so the boilerplate is written once. The two genuinely unique cases (`store_dir` wraps in `StoreDir`, `registry` appends a trailing slash) stay inline. No behaviour change.

Each invocation now takes a single field, so the macro arms no longer need `$(...)*`. Call sites are one `apply!(name);` per field, which reads more straightforwardly than a comma-separated list inside braces.

This reverts commit 76d78cd.

coderabbitai · 2026-05-08T00:52:15Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 14e2f59f-6de6-43e4-bd9c-25889a4f1c03

📥 Commits

Reviewing files that changed from the base of the PR and between b881962 and ef111bc.

📒 Files selected for processing (1)

crates/npmrc/src/workspace_yaml.rs

📝 Walkthrough

Walkthrough

The WorkspaceSettings::apply_to method is refactored to use an internal apply! macro that consolidates repetitive if let Some(v) = ... assignments for scalar and option fields. Path-valued fields and registry normalization retain their existing specialized logic.

Changes

apply_to Macro Refactoring

Layer / File(s)	Summary
Macro Pattern & Field Application `crates/npmrc/src/workspace_yaml.rs`	Internal `apply!` macro introduced to conditionally assign option fields from `WorkspaceSettings` to `Npmrc`. Specialized handling for path-valued fields (`modules_dir`, `virtual_store_dir`, `store_dir`) using `resolve` and `StoreDir::from`, and `registry` normalization with trailing slash remains intact.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A macro hop through boilerplate rain, ✨
Dedups the fields with elegant strain,
Paths and registries keep their own way,
Cleaner code now, hooray hooray! 🐰

🚥 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: refactoring the WorkspaceSettings::apply_to method by introducing a macro to reduce code repetition.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 claude/refactor-apply-to-macro-7eAbZ

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

github-actions · 2026-05-08T00:56:57Z

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.00     16.2±0.24ms   267.3 KB/sec    1.00     16.2±0.49ms   267.3 KB/sec

codecov · 2026-05-08T01:03:08Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.46%. Comparing base (b881962) to head (ef111bc).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #406      +/-   ##
==========================================
+ Coverage   82.39%   82.46%   +0.07%     
==========================================
  Files          65       65              
  Lines        3731     3672      -59     
==========================================
- Hits         3074     3028      -46     
+ Misses        657      644      -13

☔ 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.

Inlining the path-resolved fields after dropping the macro arm accidentally lost `virtual_store_dir`, so setting it in `pnpm-workspace.yaml` was silently ignored. Add the missing block alongside `modules_dir`.

Copilot

Pull request overview

This PR refactors WorkspaceSettings::apply_to in the npmrc workspace YAML loader to reduce repetitive per-field if let Some(...) assignments by introducing a small local macro for direct field copies, while keeping special-case handling for path resolution and registry normalization.

Changes:

Introduced a local apply! macro to apply many Option<T> fields onto Npmrc via direct assignment when set.
Kept explicit handling for path-valued fields (modules_dir, virtual_store_dir, store_dir) and for registry’s trailing-slash normalization.
Reordered the application flow so the bulk direct assignments happen in one consolidated block.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-05-08T01:23:11Z

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Command	Mean [s]	Min [s]	Max [s]	Relative
`pacquet@HEAD`	2.597 ± 0.286	2.379	3.380	1.08 ± 0.12
`pacquet@main`	2.408 ± 0.077	2.292	2.536	1.00
`pnpm`	6.220 ± 0.077	6.070	6.364	2.58 ± 0.09

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.5973824391,
      "stddev": 0.2857749802816158,
      "median": 2.5328784101,
      "user": 2.3888941,
      "system": 3.4230714200000003,
      "min": 2.3793476316,
      "max": 3.3801460076,
      "times": [
        2.4387035716,
        2.6324762326,
        2.5301845845999997,
        2.4622160626,
        2.4520468626,
        2.3793476316,
        2.5889391566,
        3.3801460076,
        2.5355722356,
        2.5741920456
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.4080703563999997,
      "stddev": 0.07703835571946402,
      "median": 2.3877372071,
      "user": 2.3566448,
      "system": 3.3519410199999995,
      "min": 2.2924023376,
      "max": 2.5355936216,
      "times": [
        2.3757100896,
        2.3500886415999998,
        2.5262296296,
        2.5355936216,
        2.4287058176,
        2.3993763646,
        2.2924023376,
        2.3760980496,
        2.3553211575999997,
        2.4411778546
      ]
    },
    {
      "command": "pnpm",
      "mean": 6.2201405867999995,
      "stddev": 0.07668746224546355,
      "median": 6.2211889476,
      "user": 9.039660000000001,
      "system": 4.521090320000001,
      "min": 6.0704302096,
      "max": 6.3638860886,
      "times": [
        6.1880836956,
        6.1739024246,
        6.3638860886,
        6.2270899025999995,
        6.2206718925999995,
        6.0704302096,
        6.2982680926,
        6.2024365336,
        6.2349310256,
        6.2217060025999995
      ]
    }
  ]
}

Scenario: Frozen Lockfile (Hot Cache)

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`pacquet@HEAD`	714.6 ± 37.6	668.1	808.1	1.00
`pacquet@main`	759.8 ± 39.2	703.4	812.4	1.06 ± 0.08
`pnpm`	2584.5 ± 74.3	2451.2	2694.7	3.62 ± 0.22

BENCHMARK_REPORT.json

{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.7146434838400001,
      "stddev": 0.03763542110664967,
      "median": 0.7094533765400002,
      "user": 0.25158939999999996,
      "system": 1.3603226999999998,
      "min": 0.6680517735400001,
      "max": 0.8080795475400001,
      "times": [
        0.8080795475400001,
        0.7070401895400001,
        0.6680517735400001,
        0.7119527015400001,
        0.7071889885400001,
        0.7336075375400001,
        0.7117177645400001,
        0.69023424454,
        0.7208242095400001,
        0.68773788154
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.7597708161400001,
      "stddev": 0.039236278339127094,
      "median": 0.7579173755400002,
      "user": 0.25202319999999995,
      "system": 1.3801342999999997,
      "min": 0.70338907654,
      "max": 0.8123845205400001,
      "times": [
        0.8118740955400001,
        0.8012849035400001,
        0.8123845205400001,
        0.7592656025400001,
        0.7565691485400001,
        0.7375036395400001,
        0.7474400405400001,
        0.7067653385400001,
        0.7612317955400001,
        0.70338907654
      ]
    },
    {
      "command": "pnpm",
      "mean": 2.58453596764,
      "stddev": 0.07425506148794057,
      "median": 2.58256484254,
      "user": 3.1902081999999994,
      "system": 2.2287887000000004,
      "min": 2.4511977805400003,
      "max": 2.69474663754,
      "times": [
        2.69474663754,
        2.5749855575400002,
        2.5645543625400005,
        2.59943316854,
        2.4511977805400003,
        2.5901441275400003,
        2.50922954854,
        2.6099567565400004,
        2.55746595454,
        2.6936457825400004
      ]
    }
  ]
}

claude added 3 commits May 8, 2026 00:47

refactor(npmrc): drop repetition syntax from apply! macro

76d78cd

Each invocation now takes a single field, so the macro arms no longer need `$(...)*`. Call sites are one `apply!(name);` per field, which reads more straightforwardly than a comma-separated list inside braces.

Revert "refactor(npmrc): drop repetition syntax from apply! macro"

f1d95a2

This reverts commit 76d78cd.

KSXGitHub requested a review from zkochan May 8, 2026 00:52

refactor: more concise macro

0c689f9

refactor: remove false dry

bcba8c2

fix(npmrc): restore virtualStoreDir handling in apply_to

ef111bc

Inlining the path-resolved fields after dropping the macro arm accidentally lost `virtual_store_dir`, so setting it in `pnpm-workspace.yaml` was silently ignored. Add the missing block alongside `modules_dir`.

KSXGitHub marked this pull request as ready for review May 8, 2026 01:18

Copilot AI review requested due to automatic review settings May 8, 2026 01:18

Copilot started reviewing on behalf of KSXGitHub May 8, 2026 01:19 View session

KSXGitHub enabled auto-merge (squash) May 8, 2026 01:20

Copilot AI reviewed May 8, 2026

View reviewed changes

zkochan approved these changes May 8, 2026

View reviewed changes

KSXGitHub merged commit b5962b8 into main May 8, 2026
19 checks passed

KSXGitHub deleted the claude/refactor-apply-to-macro-7eAbZ branch May 8, 2026 11:16

coderabbitai Bot mentioned this pull request May 12, 2026

refactor(config): rename pacquet-npmrc → pacquet-config and drop ini wiring #420

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(npmrc): collapse `WorkspaceSettings::apply_to` with macro#406

refactor(npmrc): collapse `WorkspaceSettings::apply_to` with macro#406
KSXGitHub merged 6 commits into
mainfrom
claude/refactor-apply-to-macro-7eAbZ

KSXGitHub commented May 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 8, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented May 8, 2026 •

edited

Loading

Uh oh!

codecov Bot commented May 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented May 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

KSXGitHub commented May 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

github-actions Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Micro-Benchmark Results

Linux

Uh oh!

codecov Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented May 8, 2026

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Scenario: Frozen Lockfile (Hot Cache)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KSXGitHub commented May 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 8, 2026 •

edited

Loading

github-actions Bot commented May 8, 2026 •

edited

Loading

codecov Bot commented May 8, 2026 •

edited

Loading