Conversation
github-project-automation
Bot
moved this to In Progress / Draft
in 🟣 [Orange-Boosted-Bootstrap] PRs Board
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Adds a new workspace package @ouds/web-migrate to integrate the OUDS Web migration CLI into the monorepo (issue #3520), including replacement rules and snapshot-based tests for HTML/CSS migrations.
Changes:
- Introduces
packages/migrateCLI package with migration engine (replace-in-file) and replacement sets forboosted,ob1, andouds. - Adds Vitest tests + fixtures/snapshots covering replacements and warning emission.
- Updates repo tooling to exclude the new non-brand workspace from SRI generation and links the workspace in the root lockfile.
Reviewed changes
Copilot reviewed 23 out of 26 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/migrate/vitest.config.js | Adds Vitest configuration for the migrate package. |
| packages/migrate/tests/source-ouds.html | HTML fixture for “from ouds” migration testing. |
| packages/migrate/tests/source-ob1.html | HTML fixture for “from ob1” migration testing. |
| packages/migrate/tests/source-boosted.html | HTML fixture for “from boosted” migration testing. |
| packages/migrate/tests/source-boosted.css | CSS fixture for migration testing. |
| packages/migrate/tests/snapshots/migrated-ouds.html | Expected migrated output snapshot (ouds). |
| packages/migrate/tests/snapshots/migrated-ob1.html | Expected migrated output snapshot (ob1). |
| packages/migrate/tests/snapshots/migrated-boosted.html | Expected migrated output snapshot (boosted). |
| packages/migrate/tests/snapshots/migrated-boosted.css | Expected migrated output snapshot (CSS). |
| packages/migrate/src/utils/warnings.mjs | Warning collection + helpers for deprecated/removed classes. |
| packages/migrate/src/utils/args.mjs | CLI arg parsing for --from. |
| packages/migrate/src/replacements/replacements.spec.mjs | Unit tests for core replacement generators. |
| packages/migrate/src/replacements/ouds.mjs | OUDS breakpoint-related replacements. |
| packages/migrate/src/replacements/ob1.mjs | OB1-specific replacements + OUDS updates. |
| packages/migrate/src/replacements/index.mjs | Builds regex arrays for replace-in-file per source set. |
| packages/migrate/src/replacements/common.mjs | Shared replacements and generators (spacing/gap/etc.). |
| packages/migrate/src/replacements/boosted.mjs | Boosted-specific replacements + OUDS updates. |
| packages/migrate/src/migrate.spec.mjs | Integration tests running the migration over fixtures and validating warnings. |
| packages/migrate/src/migrate.mjs | Thin wrapper around replace-in-file with computed replacements. |
| packages/migrate/package.json | Declares the new workspace package and CLI bin entrypoint. |
| packages/migrate/package-lock.json | Adds a per-package lockfile for migrate (new). |
| packages/migrate/index.mjs | CLI entrypoint wiring args → migrate + warning output. |
| packages/migrate/README.md | Documentation for usage/options/examples of the CLI. |
| package-lock.json | Links the new workspace and updates dependency graph. |
| build/generate-sri.mjs | Excludes packages/migrate from brand SRI generation. |
| .gitignore | Ignores packages/migrate/node_modules. |
Files not reviewed (1)
- packages/migrate/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sh.config.fatal = true | ||
|
|
||
| const BRANDS = fs.readdirSync('packages', { withFileTypes: true }).filter(file => file.isDirectory()).map(dir => dir.name) | ||
| const BRANDS = fs.readdirSync('packages', { withFileTypes: true }).filter(file => file.isDirectory() && file.name !== 'migrate').map(dir => dir.name) |
There was a problem hiding this comment.
Hard-coding file.name !== 'migrate' couples this script to a specific directory name. Since the script actually requires packages/<brand>/config.yml, consider filtering directories by the existence of config.yml (or another brand marker) instead, so adding future non-brand workspace packages won’t require updating this script again.
There was a problem hiding this comment.
Let's do that if or when we add another non-brand sub-package
|
|
||
| return options.remove ? '' : (options.replace === undefined ? args[0] : options.replace) | ||
| } |
There was a problem hiding this comment.
When options.remove is true, the replacement returns an empty string, which can leave behind empty/whitespace-only class attributes (e.g. the snapshot ends up with class=" "). Consider trimming adjacent whitespace when removing a class token, or add a post-pass to normalize class attributes (collapse spaces and turn class=" " into class="").
There was a problem hiding this comment.
This is a fair point, we could add a replacement for class=" " or something more complex to handle class="some-class ".
But right now we only remove for alert-sm and then again I'm not that confident to actually remove while we have dual-mode projects... Should we keep the remove option and handle spaces or should we not remove anything?
What do you think @vprothais?
boosted-bot
moved this from In Progress / Draft
to Need Dev Review
in 🟣 [Orange-Boosted-Bootstrap] PRs Board
Types of change
Related issues
Closes #3520
Context & Motivation
Integrate the migration tool into OUDS web monorepo
Description
See packges/migrate/README.md
Checklists
Checklist (for Core Team only)
Progression (for Core Team only)
ouds/mainfollowing conventional commitLive previews