Closes#346.
Adds a **Reporter / log events** section to `CODE_STYLE_GUIDE.md` plus a pointer in `AGENTS.md`'s cardinal-rule section so the "match pnpm" mandate explicitly covers log emissions.
The section walks a porter through the full lifecycle:
- **Finding the upstream emit** — three call shapes to grep for in `pnpm/pnpm` (`globalLogger`, `logger.debug({ name: ... })`, `streamParser.write(...)`).
- **Channel mapping** — `LogEvent` is the channels pacquet currently emits, *not* pnpm's full set; the convention for adding a new variant when an upstream channel is missing (mirror the TS shape field-for-field, add a wire-shape unit test) lives in the section. Notes the type-vs-emit-site permalink distinction: single-site channels link both, multi-site channels (`Stage`, `Progress`) only pin the type.
- **Threading the reporter** — `<R: Reporter>` generic on every fn that emits, turbofish at the production entry point, install-scoped state via `&AtomicU8`-style parameters where dedup matters (`link_file::log_method_once` cited as the example).
- **Where to put the emit** — match the upstream call site's position relative to side effects; cite the upstream permalink at a pinned SHA in the code comment.
- **Testing** — recording-fake reporter (unit-struct + `static Mutex<Vec<LogEvent>>` declared inside the test fn); sequence assertion via `matches!`. Verify the test catches the regression by temporarily commenting out the emit.
- **What not to do** — don't reformat upstream messages, don't invent channels, don't change emit granularity (cites `pnpm:fetching-progress in_progress`'s `Content-Length` + `>= 5 MB` + 500ms gate as the worked counter-example, with the pinned upstream permalink to `remoteTarballFetcher.ts`).
- **Worked example** — `pnpm:summary` in `Install::run`. Shows the upstream permalink, the pacquet emit with code comment, and the test that pins the sequence.
The doc is self-contained — no GitHub-issue back-references in the body — so a porter reads it linearly without context-switching.
Pure docs, no code changes.
* refactor: extract all inline test modules into dedicated files
Move every inline `#[cfg(test)] mod tests { ... }` block across the
workspace into external `tests.rs` files and update CODE_STYLE_GUIDE.md
to require external test files unconditionally (no threshold).
28 files extracted across 8 crates: store-dir, package-manager, fs,
package-manifest, lockfile, npmrc, cli, registry.
Closes#358.
* fix(package-manager): relocate snapshot to match new test file location
When `install::tests::should_install_dependencies` moved from the
inline `mod tests` in `install.rs` to the external
`install/tests.rs`, insta's snapshot lookup directory shifted from
`crates/package-manager/src/snapshots/` to
`crates/package-manager/src/install/snapshots/`. CI failed on every
platform because no snapshot existed at the new location. Move the
existing `.snap` file to its new home.
* test: address review feedback on extracted test files
Apply the actionable review comments on the test-extraction PR:
- save_lockfile: replace hardcoded `/nonexistent-pacquet-dir/...`
with a tempdir-based missing path so the invalid-path test is
deterministic across environments; add diagnostic message to the
`matches!` assertion.
- snapshot_dep_ref: convert `looks_like_alias_rules` to a
table-driven `assert_eq!` form with per-case logging.
- npmrc/workspace_yaml: compare `StoreDir` values directly instead
of `display().to_string()`, which produces backslashes on Windows
and would fail the `apply_overrides_npmrc_defaults` test there.
- package-manager/install: clear the static `EVENTS` mutex at the
start of `install_emits_pnpm_event_sequence` so retries don't
inherit stale events; add per-path logging before the
`is_symlink_or_junction` / `exists` / `is_dir` assertions.
- package-manager/link_file: same `EVENTS.clear()` defense in
`log_method_once_emits_first_call_per_method_only`; add
diagnostics before nlink and symlink-metadata assertions.
- package-manager/build_snapshot: log the relevant fields before
the `matches!` and `is_none` assertions.
- package-manager/install_package_from_registry: log
`virtual_store_path` state before the `is_dir` assertion.
- package-manifest: assert the actual return value of
`manifest.script("test", false)` (was `Some("echo")`) and
`manifest.script("invalid", true)` (was `None`); add `dbg!` /
`eprintln!` diagnostics around the bare `assert!` checks per
CODE_STYLE_GUIDE.md.
- store-dir/check_pkg_files_integrity: add `dbg!(&result)` and
path-existence logging before each non-`assert_eq!` assertion
across all 11 tests.
---------
Co-authored-by: Zoltan Kochan <z@kochan.io>
* docs(style): warn against referencing private items from public docs
* docs(style): rewrite doc-comment rule to match writing style
* docs(style): correct doc-comment visibility framing
* docs(style): drop misleading comment on private helper in example
* docs(style): clarify that the rule targets more-private references
---------
Co-authored-by: Claude <noreply@anthropic.com>
* docs(style): require intra-doc links for identifier mentions
Add a CODE_STYLE_GUIDE.md section telling contributors (and AI agents)
to turn identifier mentions inside `///` and `//!` doc comments into
rustdoc intra-doc links when the identifier is reachable from scope.
Intra-links give readers one-click navigation and let rustdoc surface
broken-link warnings when items are renamed or removed.
* docs(style): tighten intra-doc link guidance
Drop the redundant `//!` callout — saying "doc comment (`///` or `//!`)"
once at the top is enough; the second example only restated the rule.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* docs: fix bugs and typos in CODE_STYLE_GUIDE.md
- Correct push_path/push_back function name mismatch
- Fix non-compiling call sites in the &Path example so the snippet
type-checks against the stated signature
- Add missing `move` and trailing `;` to tokio::task::spawn examples
so they compile
- Rename "Cloning an atomic counter" section to "Cloning `Arc` and `Rc`"
since Arc/Rc are reference counters, not atomic counters
- Fix missing semicolon in an assert_eq! example
- Grammar fixes ("could easily be refactored", "Explicitly marking
the cloned type aids", "as more pull requests are reviewed")
* docs: add CONTRIBUTING.md and AGENTS.md
Adapted from KSXGitHub/parallel-disk-usage's CONTRIBUTING.md, with
pacquet-specific changes:
- No version-release exception in the commit message convention
- Setup and automated-check sections rewritten around `just init`,
`just install`, and `just ready` instead of `test.sh`
- Dropped the squashfs/fuse external-dependency section (not relevant
to pacquet); kept the registry-mock note instead
- Examples reworked to use pacquet types (manifests, store, etc.)
- Cross-links between CONTRIBUTING.md and CODE_STYLE_GUIDE.md so the
two documents are discovered together
AGENTS.md instructs AI agents to follow both guides.
* docs: pass &mut my_list in push_path examples
Addresses copilot review comment on PR #255: `push_path` takes
`&mut Vec<PathBuf>`, so the call sites need `&mut my_list` rather
than `my_list` to compile.
* chore: temporarily remove AGENTS.md before merging main
main already has an AGENTS.md (added in #268) with content unrelated
to my edits. Removing the duplicate so the merge from main pulls in
that file cleanly; the AI-guide references will be re-added on top.
* docs(agents): point at CONTRIBUTING.md and CODE_STYLE_GUIDE.md
Adds an explicit "Follow the project guides" section near the top of
AGENTS.md and lists CONTRIBUTING.md alongside CODE_STYLE_GUIDE.md in
the repo-layout overview, so both guides are surfaced as required
reading.
* docs: move code-style sections from CONTRIBUTING.md to CODE_STYLE_GUIDE.md
Per option A of the contributing/style split discussion: keep
CONTRIBUTING.md focused on process (commit message format, writing
style, setup, automated checks), and have CODE_STYLE_GUIDE.md own
every code-level convention.
Moved into CODE_STYLE_GUIDE.md (under the existing `## Guides`
section, in thematic order):
- Module Organization
- Import Organization
- Derive Macro Ordering
- Generic Parameter Naming
- Variable and Closure Parameter Naming (with the single-letter rules)
- Trait Bounds
- Pattern Matching
- Using `pipe-trait` (with when-to-use and when-not-to-use)
- Error Handling
- Conditional Test Skipping (`#[cfg]` vs `#[cfg_attr(..., ignore)]`)
- Unit test file layout (was the top-level `## Unit Tests` section)
The intra-doc link from "Where the external file sits" to
"Module Organization" still resolves because both sections now live
in the same file.
Updated `AGENTS.md`'s descriptions for both guides to reflect the new
split. Per the review thread on this PR, no other prose in
`AGENTS.md` was rewritten for writing-style conformance; that is
deferred to the maintainer.
* docs(style): drop "Derive Macro Ordering" section
The codebase does not follow the rules this section prescribed, so
the section was aspirational rather than descriptive of pacquet's
actual conventions:
- "Split across multiple #[derive(...)] lines" — never done. Across
52 files with derives, zero stack two #[derive(...)] attributes.
- Prescribed standard → comparison → Hash → derive_more order —
frequently violated. The dominant pattern places Display right
after Debug (e.g. crates/lockfile/src/comver.rs:8,
crates/lockfile/src/pkg_ver_peer.rs:12).
- #[cfg_attr(feature = "...", derive(...))] split — zero
occurrences anywhere in crates/.
The codebase's actual convention is too loose to codify usefully
(one #[derive(...)] line, Debug first when present), so the section
is removed rather than weakened to a near-tautology.
* docs(style): use .filter on both sides of the parameter-naming example
The pair was meant to contrast the closure parameter name (`entry`
vs. `x`), but using `.filter_map` on the good side and `.filter` on
the bad side muddled that with a method-choice difference. The
mismatch was carried over from the upstream parallel-disk-usage
guide. Use `.filter` on both sides so the example isolates the rule
under discussion.
* docs(style): fix four porting quirks in CODE_STYLE_GUIDE.md examples
Sweep for the same class of issue as the .filter / .filter_map
mismatch (#255, KSXGitHub review): examples that were degraded when
ported from upstream parallel-disk-usage.
- show_path with `path: &Path`: passing `my_path_buf` directly
doesn't compile because `PathBuf` does not auto-coerce to `&Path`
in argument position. Use `&my_path_buf` (same fix family as the
earlier push_path call sites).
- "Avoiding deeply nested function calls": the pre-pipe version was
a single non-nested call, so the rule's motivation wasn't visible.
Replaced with a genuinely nested constructor chain that piping
flattens.
- "pipe_as_ref" example lost its contrast in the port; restored the
"without pipe" companion line so the rule's payoff is shown.
- Picked a custom-looking pacquet function name (is_within_store)
for that example instead of `Path::exists`, since nobody uses
`path_buf.pipe_as_ref(Path::exists)` over `path_buf.exists()`.
- Renamed `rows.zip(cols)` to `left_indices.zip(right_indices)` so
the `(i, j)` closure parameters line up with the rule about index
variables.
* docs: address review-agent findings on the docs branch
- AGENTS.md: drop the stale "derives" topic from both descriptions
of CODE_STYLE_GUIDE.md, since the Derive Macro Ordering section
was removed in 06c08ff for not matching the codebase.
- AGENTS.md: replace the inaccurate "pre-commit checks" wording in
the repo-layout entry for CONTRIBUTING.md. The pre-push hook only
runs format checks; the full `just ready` suite is a manual step
before submitting, not an automated pre-commit hook.
- CODE_STYLE_GUIDE.md: fix subjunctive grammar — "should a test
fails" should be "should a test fail" (predates the recent move,
but caught while reviewing the section).
* Revert "docs(style): fix four porting quirks in CODE_STYLE_GUIDE.md examples"
This reverts commit 0d1e4348f0e8f66e552a7d8eb03db701c6fc0fcd.
* docs(style): make the pipe_as_ref example actually mid-chain
The text says "to pass a reference mid-chain", but the example was
a single `path_buf.pipe_as_ref(Path::exists)` call with no chain at
all. Replaced with a four-step chain that has methods before and
after `pipe_as_ref`, so the rule's wording matches the demonstration.
Audited every other pipe example in the section: each one is either
already part of a chain (dependencies, into_iter, summarize, etc.)
or is intentionally chain-free in the "When NOT to use pipe"
section, so no other rectification was needed.
Addresses KSXGitHub review on PR #255.
* docs(contributing): describe `just install` generically
Replace "Install the registry-mock dependencies" with "Install the
test dependencies". The previous wording named an internal component
(registry-mock) and would drift if `just install` ever did more or
different work. The new wording defers to `just install` as the
source of truth for what gets installed.
Per KSXGitHub review on PR #255.
* docs(style): drop `idx` from the index-naming alternatives
Per @zkochan review on PR #255: reduce the number of alternatives
listed for index variables. Keep `index` and `*_index` (such as
`row_index`); drop `idx`.
Applied to both places where the triple appeared: the "Conventional
single-letter names" bullet and the "Index variables" rule, so the
two stay consistent.
---------
Co-authored-by: Claude <noreply@anthropic.com>
