Repository navigation
docs: add Ruby, Go, Dart, and PHP rows to dependency-scope mapping table - #607
Janil-ship-it wants to merge 3 commits into
Conversation
… in yarn.lock Signed-off-by: Janil-ship-it <jjain2603@gmail.com>
Signed-off-by: Janil-ship-it <jjain2603@gmail.com>
Signed-off-by: Janil-ship-it <jjain2603@gmail.com>
vibgrate-team
left a comment
There was a problem hiding this comment.
Thanks for picking up #555 and for the DCO-signed commits.
We're going to need a few changes before this can go further — please don't take it as a reflection on the effort.
1. Scope: engine changes don't belong on this PR
The title and description are docs-only (Ruby / Go / Dart / PHP rows for docs/sbom-dependency-scope.md), but the branch also changes src/engine/lockfile.ts and src/engine/lockfile.test.ts with Yarn Berry workspace / patch / alias handling. That engine work is a separate concern (it previously lived on #516, which you closed). Please drop those two files from this PR so this one stays a pure docs change for #555. If you still want the Yarn Berry lockfile behaviour, open (or keep) a dedicated engine PR for it — #608 currently looks like it carries the same engine diff as well, so worth consolidating there rather than repeating it.
2. Dart and PHP rows are out of date vs current main
main already documents both signals correctly, and the scanners emit development rows today:
pubspec.yamldev_dependencies— already a table row: sectiondevDependencies, exported, CycloneDXscope=excluded, SPDXDEV_DEPENDENCY_OF.composer.jsonrequire-dev— already a table row with the same shape, andcomposer.lockpackages/packages-devare also documented (lockfile is expanded).
The new rows say Dart / PHP dev deps are "Parsed, then removed by allDeps.filter(d => !d.isDev)" and that composer.lock is not expanded. That matched an older gap description on #555, but it no longer matches main (see also test/sbom-dart-dev-dependencies.test.ts and test/sbom-php-require-dev.test.ts). Adding those rows would duplicate the existing signals with incorrect text.
Please remove the Dart and PHP additions from this PR. #555's acceptance for those two ecosystems is already satisfied on main; we can update the issue text to match.
3. Ruby and Go rows — keep, but re-check against today's scanners
Gemfile group / gemspec add_development_dependency and go.mod // indirect / go.sum are still missing from the mapping table, so those four rows are welcome — after a fresh read of:
src/core-open/scanners/ruby-scanner.ts(parseGemfilegroup classification: a block is development/test only when every symbol is:development/:dev/:test; mixed groups stay production)src/core-open/scanners/go-scanner.ts(// indirectis normally dropped from the scan, with a narrow exception for modules that provide atoolpackage, which land asdevDependencies)
Please phrase the rows for current behaviour only, in the same voice as the neighbouring table entries, and rebase onto current main so the insertion point sits with the other ecosystem rows rather than after the Gradle block on a stale base (this branch is ~42 commits behind).
Summary
- Drop
lockfile.ts/lockfile.test.tsfrom this PR. - Drop the Dart and PHP docs rows (already covered correctly on
main). - Keep / tighten the Ruby and Go rows against the scanners above, rebased on
main.
Happy to take another look once that's pushed. No action needed on #609 from this review — that one touches different rows in the same file.
Pull Request
Summary
Adds missing ecosystem mapping rows for Ruby (
Gemfile/gemspec), Go (go.mod/go.sum), Dart (pubspec.yaml), and PHP (composer.json) to the main mapping table indocs/sbom-dependency-scope.md.Related issues
Closes #555
Checklist
pnpm testpassespnpm lintis cleangit commit -s, DCO)