Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
70d9348 to
387d7e3
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70d9348bde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
peter-jerry-ye
left a comment
peter-jerry-ye
left a comment
There was a problem hiding this comment.
The declaration/resolution split and backend-specific dependency graphs are justified. Requesting changes for three P2 issues: duplicate active imports must be rejected before graph insertion, import conversion needs one authoritative normalization path, and planner tests must exercise the production backend-selection policy. There is also a non-blocking naming cleanup noted inline.
The import rule should be explicit: multiple blocks may be active, but each imported package may appear only once per backend within each import kind, regardless of alias. Disjoint conditions and different import kinds remain valid.
387d7e3 to
9b9b745
Compare
9b9b745 to
9d07c31
Compare
7e192a7 to
01240da
Compare
9d07c31 to
1858849
Compare
1858849 to
eca993f
Compare
eca993f to
fbd539d
Compare
fbd539d to
27db5b9
Compare
This branch has not been deployed
2 participants
Summary
Adds backend-conditional imports to
moon.pkgfor regular, blackbox-test, and whitebox-test imports. Conditions supporttarget = "...",all(...),any(...),not(...),true, andfalse, with clearer diagnostics for misplaced#cfgattributes and unsupported conditions.Package discovery happens before backend selection. Dependency resolution produces complete
PackageRelationsfor each requested backend, which is shared by planning, lowering, metadata, package-graph export, and virtual-package handling. For example,moon check --target nativeignores broken JS-only imports, whilemoon check --target js,nativefails if either requested backend cannot resolve. Run planner tests use the production package and backend selection path.Builds on the merged parser, normalization, direct DSL-to-MoonPkg conversion, and discovery/resolution split. Uses the merged
DiscoveredProjectandResolvedProjectnaming, with coverage configuration retained by discovery. Repeated import blocks combine in declaration order. Repeated packages within one import kind remain valid and warn only when their conditions overlap. Legacy import options override the corresponding blocks before warnings; aliases and import-all settings retain their existing behavior. Legacymoon.pkg.jsonimports remain unconditional.Includes package-model, dependency-graph, planner, and CLI coverage, plus English and Chinese documentation.
Validation
Rebased onto
mainate46d2ed38(including merged #2241, #2242, and #2243). The final update from86c63700echanges onlymoonrun;git range-diffconfirms all five PR patches are unchanged.Validation before the final runtime-only base update:
cargo test --locked -p moonutil -p moonbuild-rupes-recta— 407 tests passed; one existing doctest ignored.cargo test --locked -p moon --bin moon --test mod -- tests::planner filter::tests watch::prebuild_output::tests conditional_imports major_version_modules_coexist_in_projects_and_scripts test_wbtest_coverage— 59 unit tests and 9 CLI tests passed, including Wasm execution for versioned imports in projects and scripts, plus coverage execution.cargo clippy --workspace --exclude moonrun --locked --all-targets --all-features -- -D warningscargo fmt --all -- --checkgit diff --checkMOONBIT_NEW_NATIVE=0 moon test app --target native --dry-runon the conditional-import fixture.After the final base update, reran
cargo test --locked -p moon --test mod -- major_version_modules_coexist_in_projects_and_scripts test_wbtest_coverage: both integration tests passed, covering ordinary project/script Wasm execution and coverage. Formatting and diff checks also passed again.Native/C checks used dry-runs only; the full workspace test suite was not rerun.
Actual Wasm execution of the conditional-import fixture was attempted, but the installed compiler (
moonc v0.10.14+7d59c7ec9, 2026-09-18) rejects#cfginmoon.pkg. Compiling and formatting conditional manifests still requires compiler/formatter support for that syntax. Ordinary project and script Wasm execution passed as described above.Metadata