Skip to content

docs: add Ruby, Go, Dart, and PHP rows to dependency-scope mapping table - #607

Open
Janil-ship-it wants to merge 3 commits into
vibgrate:mainfrom
Janil-ship-it:docs/sbom-scope-table
Open

Janil-ship-it wants to merge 3 commits into
vibgrate:mainfrom
Janil-ship-it:docs/sbom-scope-table

Conversation

@Janil-ship-it

Copy link
Copy Markdown

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 in docs/sbom-dependency-scope.md.

Related issues

Closes #555

Checklist

  • pnpm test passes
  • pnpm lint is clean
  • Docs updated where behavior changed
  • Commits use Conventional Commits and are signed off (git commit -s, DCO)

… 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 vibgrate-team 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.

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.yaml dev_dependencies — already a table row: section devDependencies, exported, CycloneDX scope = excluded, SPDX DEV_DEPENDENCY_OF.
  • composer.json require-dev — already a table row with the same shape, and composer.lock packages / packages-dev are 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 (parseGemfile group 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 (// indirect is normally dropped from the scan, with a narrow exception for modules that provide a tool package, which land as devDependencies)

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.ts from 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.

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 Ruby, Go, Dart and PHP rows to the dependency-scope mapping table

2 participants