build: streamline by removing unneeded build deps#25733
build: streamline by removing unneeded build deps#25733tylerbutler merged 4 commits intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR streamlines the build process by removing redundant API extraction dependencies from the build pipeline, since API validation is already handled through the lint process.
Key Changes:
- Removed
build:api-reportsandbuild:docsfrom the main build task dependencies - Removed
apidependency from thecompiletask - Removed API extractor tasks from test build dependencies (
build:test,build:test:cjs,build:test:esm)
fluidBuild.config.cjs
Outdated
| "lint", | ||
| "build:api-reports", | ||
| "build:docs", | ||
| // API validation happens through lint → check:exports → api |
There was a problem hiding this comment.
It seems odd to me that the lint step would need/run the API extractor steps that generate the API reports. I get the export checks being a part of lint, but I don't think the report generation steps belong there. Seems like they should be a dependency of build and not of lint.
There was a problem hiding this comment.
Removing it here seems like a hack to make it skipped when we run with --nolint by making everything that depends on it part of lint.
Removing it from compile below should be sufficient to relax the build dependency order and remove it from --task compile builds.
Leaving it here might be better.
There was a problem hiding this comment.
We should make out compile CI task run just the compile step, then have a separate job (in parallel with tests) that run everything else, which from this.
That suggests maybe there is some logical grouping includes all of "check:format", "lint", "build:api-reports", "build:docs", "build:manifest", "build:readme".
I could see grouping "check:format" into lint, and the rest into "build:docs" if we wanted less top level categories here.
There was a problem hiding this comment.
We should make out compile CI task run just the compile step, then have a separate job (in parallel with tests) that run everything else, which from this.
That suggests maybe there is some logical grouping includes all of "check:format", "lint", "build:api-reports", "build:docs", "build:manifest", "build:readme".
I could see grouping "check:format" into lint, and the rest into "build:docs" if we wanted less top level categories here.
If we're talking task naming and grouping overall, I suggest using this PR that has some concrete all-up proposals: #25730 (comment)
There was a problem hiding this comment.
It seems odd to me that the lint step would need/run the API extractor steps that generate the API reports. I get the export checks being a part of lint, but I don't think the report generation steps belong there. Seems like they should be a dependency of build and not of lint.
Fixed in latest.
Josmithr
left a comment
Josmithr
left a comment
There was a problem hiding this comment.
Thanks for doing this!!
|
This is incorrect. api-extractor reports should not block compilation but the API entrypoint files (which are under the |
This partially reverts "build: streamline by removing unneeded build deps (microsoft#25733)" (commit ee07c75). It lets "compile" task only depend on ancestor "api" and preserves the commented out line removal. To make the build more efficient, API entrypoint generation should be made distinct from api-extractor reports.
This partially reverts "build: streamline by removing unneeded build deps (#25733)" (commit ee07c75). It lets "compile" task only depend on ancestor "api" and preserves the commented out line removal. To make the build more efficient, API entrypoint generation should be made distinct from api-extractor reports.
This partially reverts "build: streamline by removing unneeded build deps (microsoft#25733)" (commit ee07c75). It lets "compile" task only depend on ancestor "api" and preserves the commented out line removal. To make the build more efficient, API entrypoint generation should be made distinct from api-extractor reports.
…rosoft#25733)" This reverts commit ee07c75 except for a comment removal. Adds note of why "api" should be a part of "compile".
The api-extractor tasks should not block compilation. They are already run as part of lint, so this change just removes the unneeded deps.