Skip to content

docs: add runnable Composer manifest walkthrough - #24

Open
shoaibyazdani wants to merge 1 commit into
fly1d:mainfrom
shoaibyazdani:docs/run-composer-manifest-walkthrough
Open

docs: add runnable Composer manifest walkthrough#24
shoaibyazdani wants to merge 1 commit into
fly1d:mainfrom
shoaibyazdani:docs/run-composer-manifest-walkthrough

Conversation

@shoaibyazdani

@shoaibyazdani shoaibyazdani commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

docs: add runnable Composer manifest walkthrough

Add a copy-pasteable Composer manifest reduction walkthrough to
docs/EXAMPLES.md. Reuses the existing benchmarks/composer-package fixture
and its Python oracle, runs from the repository root with the temporary
output parent outside the fixture, validates the exported payload with
report validate --payload --json, and shows the independent oracle rerun
that exits 1 with ORIGINAL_FAILURE in command output. Calls out the
no-PHP / no-Composer / no-install / no-package-index / no-repository /
no-lifecycle-scripts / no-network boundary and the descriptive Python-oracle
boundary, and links the fixture README plus SECURITY.md.

Closes #22

Observed check results

Both acceptance checks from the issue were re-run from the repository root
on this branch after rebasing onto the current upstream/main. Output
captured verbatim.

python3 scripts/check_docs.py:

Documentation check passed (47 Markdown file(s)).

Exit status: 0.

python3 benchmarks/run_offline.py --only composer-package:

PASS composer-package

offline benchmarks: 1 passed, 0 skipped, 0 failed

Exit status: 0.

@shoaibyazdani
shoaibyazdani requested a review from fly1d as a code owner September 3, 2026 08:58

@fly1d fly1d left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for the focused contribution. I independently ran both acceptance checks and the full walkthrough: the reduction produced the documented two files, exact fingerprint validation passed, and the independent oracle exited 1. One correction is on us: Issue #22 said stderr, but benchmarks/composer-package/reproduce.py uses plain print(), so ORIGINAL_FAILURE is emitted on stdout. Please change the new section to say command output (preferred, since stream placement is not the oracle contract) or stdout; do not modify the fixture. Please also update or rebase this branch onto current main and keep the recently added Node walkthrough, because the PR currently conflicts in docs/EXAMPLES.md. Then rerun the two acceptance commands. Everything else in the walkthrough matched the observed behavior and intended scope.

@fly1d

fly1d commented Sep 3, 2026

Copy link
Copy Markdown
Owner

One small precision from the second independent review: Issue #22 also calls out Composer repository access. Please change the boundary wording to say that the fixture does not access a package index or the repositories declared in the manifest. Mentioning the two removed repository entries is optional. After rebasing, please also update the PR description to correct stdout and the observed documentation-file count. These are wording updates only; the implementation and fixture should remain unchanged.

Add a copy-pasteable Composer manifest reduction walkthrough to
docs/EXAMPLES.md. Reuses the existing benchmarks/composer-package fixture
and its Python oracle, runs from the repository root with the temporary
output parent outside the fixture, validates the exported payload with
'report validate --payload --json', and shows the independent oracle rerun
that exits 1 with ORIGINAL_FAILURE on stderr. Calls out the no-PHP /
no-Composer / no-install / no-package-index / no-lifecycle-scripts /
no-network boundary and the descriptive Python-oracle boundary, and links
the fixture README plus SECURITY.md.

Closes fly1d#22
@shoaibyazdani
shoaibyazdani force-pushed the docs/run-composer-manifest-walkthrough branch from 4ab9de9 to acf5ba1 Compare September 3, 2026 15:44
@shoaibyazdani

Copy link
Copy Markdown
Contributor Author

Thanks for the careful second pass. All five points addressed:

  1. stdout correction — the independent-rerun paragraph now says ORIGINAL_FAILURE is emitted in command output, with a clarifying note that the fixture's Python oracle uses a plain print() call, so the marker appears on stdout rather than stderr. Treating it as command output keeps the description stream-agnostic.

  2. Repository boundary — opening paragraph now also explicitly states the fixture deliberately does not contact the declared repositories or fetch from any package index, alongside the existing no-PHP / no-Composer / no-install / no-lifecycle-scripts / no-network boundaries.

  3. Rebase + keep Node walkthrough — rebased docs/run-composer-manifest-walkthrough onto the current upstream/main (df942d2). Upstream had already merged the Node walkthrough (PR docs: add a runnable Node package manifest walkthrough #20), so the Composer section now sits between Node and Cargo in docs/EXAMPLES.md, which is the natural placement. No conflict remains.

  4. Rerun acceptance checks — both pass on the rebased branch:

    • python3 scripts/check_docs.py → "Documentation check passed (47 Markdown file(s))." exit 0
    • python3 benchmarks/run_offline.py --only composer-package → "PASS composer-package / offline benchmarks: 1 passed, 0 skipped, 0 failed" exit 0
  5. PR description updated — current file count (47, was 44 before upstream merge) and corrected wording (the title and body did not mention stderr directly; only the EXAMPLES.md section did, which is now fixed).

Branch force-pushed as commit acf5ba1 (79 insertions, up from 75). Ready for re-review.

@fly1d

fly1d commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Thanks again — the contribution itself is now ready from the content review, and all 10 checks on the previous head are green.

main has advanced since that rebase and is now at 51632c8, so GitHub reports this PR as behind. Could you please rebase once more onto the current main and force-push the branch? No additional content changes are requested. Once the refreshed CI is green, I will do the final review and merge.

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.

docs: add a runnable Composer manifest walkthrough

2 participants