Migrate cloud-foundry-tools-api (@sap/cf-tools) into the monorepo - #615
Merged
Merged
Conversation
…ols-api Raw relocation of @sap/cf-tools v3.3.1 into the monorepo (excludes the generated typedoc docs/). No functional changes; later commits wire it into the pnpm toolchain. Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
- extend root tsconfig.base.json; drop options covered by base - prune devDeps root already owns; keep runtime deps + @types/comment-json, @types/properties-reader - local .mocharc.js (extends root) + nyc.config.js: compiled-test layout like vscode-mta-tools, coverage at default .nyc_output so root merge-coverage finds it - delete per-package eslint/prettier config (root governs) - CHANGELOG.md -> CHANGELOG.old.md (Changesets owns the fresh one) - README hygiene + project-level README/.gitignore; exclude generated docs/ - pin consumer projects/vscode-mta-tools to 3.3.1 (cfGetTarget/ITarget API-identical across 2.0.1->3.3.1 on the used surface; verified) Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
Isolated formatting-only commit (monorepo Prettier reformats the migrated files). Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
- per-project eslint overrides disabling only the rules that fire on the migrated library + chai tests (eslint-comments/*, no-explicit-any, no-unsafe-*, no-unused-expressions, no-redundant-type-constituents); mirrors vscode-mta-tools - update lockfile: cf-tools importer (comment-json/lodash/properties-reader/url + @types) and vscode-mta-tools consumer repin to 3.3.1 Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
- untrack per-package .reuse/ + LICENSES/ (force-added past root .gitignore; scripts/legal-copy.js regenerates them at build, and no other workspace package commits them). Kept on disk + listed in package.json files (injected at pack time). - drop @types/comment-json: comment-json@4.2.5 ships its own type definitions, so the @types stub is redundant (npm-flagged deprecated). - ignore + clean nyc's default ./coverage output dir. Reconciled two independent reviewers; rejected as non-issues (verified against repo): @types/lodash 'missing' (root-owned, mta precedent omits it), strict:false 'fabricated precedent' (guided-development/tsconfig.base.json IS strict:false), changeset-status 'not clean' (expected changeset-free migration-PR state). Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
Add two small coverage cases for branches that the source repo allowed below 100% but the monorepo root coverage merge enforces at 100%: - post-spawn cancellation callback in Cli.execute - service plan response with no matching included service offering Also replace the default no-op cancellation callback with lodash's constant helper so it is not counted as an uncallable inline function in source-mapped coverage. Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
Contributor
Build ReportPlease note:
|
jacob-kreyenbuehl
marked this pull request as ready for review
September 15, 2026 00:13
Resolve conflicts in .eslintrc.js (keep both cf-tools and feature-toggle-node TS override blocks) and regenerate pnpm-lock.yaml from main + cf-tools package. Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
Contributor
|
Why do we have a |
…ry-tools-api # Conflicts: # pnpm-lock.yaml
Contributor
Author
|
Hi Deeksha. Yes, it is on purpose.
This is the settled convention. All migrated projects use it: feature-toggle-node, inquirer-gui, task-explorer, vscode-logging, xml-tools, code-snippet, guided-development and yeoman-ui. We keep it here so all projects stay identical, and so a project can add more packages later without moving files. |
rolanbadrislamov
self-requested a review
September 23, 2026 12:28
rolanbadrislamov
approved these changes
Sep 23, 2026
jacob-kreyenbuehl
added a commit
that referenced
this pull request
Sep 23, 2026
The migration (#615) shipped changeset-free. Add the post-merge patch changeset so Changesets releases @sap/cf-tools from the new monorepo location. Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
jacob-kreyenbuehl
added a commit
that referenced
this pull request
Sep 23, 2026
The migration (#615) shipped changeset-free. Add the post-merge patch changeset so Changesets releases @sap/cf-tools from the new monorepo location. Signed-off-by: Jacob Kreyenbuehl <jacob.kreyenbuehl@sap.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Migrates the standalone
SAP/cloud-foundry-tools-apipackage (@sap/cf-tools, v3.3.1) into this monorepo underprojects/cloud-foundry-tools-api/packages/cloud-foundry-tools-api/, and repins the in-repo consumervscode-mta-toolsfrom@sap/cf-tools@2.0.1to the migrated3.3.1.Why
Consolidates the BAS OSS libraries into the monorepo so they share one toolchain, CI, and release flow (Changesets). Continues the effort started with
feature-toggle-node(#612).Behaviour
Mostly a relocation. The original
src/+tests/lift was mechanically verified as byte-for-byte identical to the source repo after Prettier normalization. After CI, one tiny non-behavioral source substitution (_.constant("")for an equivalent default callback) plus two focused tests were added so the migrated package satisfies the monorepo root 100% merged-coverage gate. The generated typedocdocs/(93 files) is intentionally not migrated.Reviewer's guide
The commits are ordered so the mechanical move is isolated from the human decisions:
move— raw relocation of the package (excludes generateddocs/). No logic changes.wire— pnpm toolchain integration:package.json(name/version/files unchanged from published tarball; runtime deps kept; root-owned devDeps pruned),tsconfig.jsonextends root base, local.mocharc.js/nyc.config.js, project README/.gitignore, consumer pin bump.style— Prettier reflow only.eslint + lockfile— per-project ESLint overrides (only the rules that fire on the legacy library + chai tests) and lockfile update.review fixes— addressed independent review findings (see below).coverage gate— added two focused tests + equivalent default callback helper to satisfy root merged coverage at 100%.The three decisions worth your attention:
tsconfig.jsonsetsstrict: false(re-enablingnoImplicitAny/noImplicitThis/alwaysStrict). The source repo compiled under partial strict (nostrictNullChecks); the monorepo base is fullstrict: true. Forcing full strict would require rewriting library logic, which a relocation must not do. This mirrors the existingprojects/guided-development/tsconfig.base.json(strict: false). Tightening to full strict is left as a follow-up.outDirisout/(notlib/dist) to preserve the exact published tarball layout (main: out/src/index.js). The monorepo already tolerates mixed conventions across packages.vscode-mta-toolsuses onlycfGetTarget+ITarget, both byte-identical across the versions; the package compiles, tests, and produces its VSIX green with the migrated version.Coverage / release:
nycruns on compiled JS (include: ["out/src/**"],excludeAfterRemap: false) with the source's 99/99/98/99 gate, writing to the default.nyc_outputso the rootmerge-coverage.jspicks it up. No changeset is included (migration PRs ship changeset-free; the release graph stays clean because the consumer pin was bumped in the same PR).Independent review: this PR was reviewed by three independent model passes before being marked ready (Opus 4.8 + GPT 5.5 + GPT 5.5 current-branch pass), plus a narrow GPT 5.5 review of the final coverage-gate commit after CI exposed the merged-coverage issue. Fixed: untracked the per-package
.reuse/+LICENSES/(they are regenerated at build byscripts/legal-copy.jsand were force-added past.gitignore); dropped the redundant deprecated@types/comment-jsonstub (the package ships its own types). Reviewer claims that were verified as non-issues and rejected:@types/lodash"missing" (root-owned; thevscode-mta-toolsprecedent omits it), thestrict:falseprecedent (confirmed real), and changeset-status "not clean" (expected changeset-free state).CI follow-up: the first draft CI run failed only at root
coverage:mergebecause cf-tools' source-level 99.x coverage was now included in the monorepo's global 100% merged gate. Fixed by adding legitimate coverage for the uncovered cancellation and service-offering fallback branches; local@sap/cf-toolsCI reports 187 tests and 100/100/100/100 coverage, andscripts/merge-coveragepasses with cf-tools output.