Skip to content

chore: refactor common.py CLI parsing and constants !macos !windows !js - #89

Merged
eagleoflqj merged 3 commits into
fcitx-contrib:masterfrom
WhiredPlanck:arg-parse
Aug 22, 2026
Merged

eagleoflqj merged 3 commits into
fcitx-contrib:masterfrom
WhiredPlanck:arg-parse

Conversation

@WhiredPlanck

@WhiredPlanck WhiredPlanck commented Aug 22, 2026 •

Copy link
Copy Markdown
Contributor

!macos !windows !ios !harmony !js

Summary by CodeRabbit

  • Enhancements
    • Improved platform selection for builds and deployments with clearer, validated platform commands.
    • Added validation for supported architecture and SDK combinations.
    • Standardized platform-specific defaults, deployment settings, and reproducibility options.
    • Updated macOS and iOS packaging to use explicit version settings for more predictable releases.
    • Preserved required locale data in relevant packaged components.
    • Restricted automated release actions to eligible pull requests, improving release safety.

@coderabbitai

coderabbitai Bot commented Aug 22, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 22ab9305-88a9-40fb-b22a-1b03f1695620

📥 Commits

Reviewing files that changed from the base of the PR and between d0b93ce and f36d7a2.

📒 Files selected for processing (3)
  • scripts/common.py
  • scripts/iso-codes.py
  • scripts/xkeyboard-config.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The build scripts now parse platform options centrally, initialize shared platform configuration, use it for project selection and deployment versions, and control locale preservation through Builder. The JavaScript workflow prevents releases from forked pull requests.

Changes

Platform Build Flow

Layer / File(s) Summary
Platform argument parsing and initialization
scripts/common.py
Adds required platform subcommands and validates architecture or SDK options. Centralizes platform defaults, Cargo targets, postfixes, reproducibility variables, and deployment settings.
Build selection and deployment integration
scripts/build.py, scripts/common.py
Uses shared PLATFORM for project selection and explicit macOS or iOS version constants for CMake configuration.
Locale packaging control
scripts/common.py, scripts/iso-codes.py, scripts/xkeyboard-config.py
Adds the keep_locale builder option and enables it for iso-codes and xkeyboard-config.

Release Workflow Gating

Layer / File(s) Summary
Fork-aware release guard
.github/workflows/js.yml
Restricts the Release step to pull requests from the same repository.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to f36d7

The refactor changes the command-line invocation contract and may affect locale packaging or macOS deployment compatibility if callers, directory assumptions, or version settings are not aligned. The PR is mergeable with explicit owner awareness and follow-up on these bounded integration and build risks.

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant common.py
  participant build.py
  participant Builder
  participant CMake
  CLI->>common.py: provide platform subcommand and options
  common.py->>common.py: validate options and initialize platform settings
  common.py->>build.py: expose PLATFORM
  build.py->>build.py: select platform_projects[PLATFORM]
  build.py->>Builder: configure locale preservation
  Builder->>CMake: configure explicit macOS or iOS version
Loading

Suggested reviewers: eagleoflqj

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: refactoring CLI parsing and constants in common.py.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@eagleoflqj eagleoflqj changed the title chore: refactor common.py CLI parsing and constants chore: refactor common.py CLI parsing and constants !macos !windows !ios !harmony Aug 22, 2026
@eagleoflqj eagleoflqj changed the title chore: refactor common.py CLI parsing and constants !macos !windows !ios !harmony chore: refactor common.py CLI parsing and constants !macos !windows !js Aug 22, 2026
@eagleoflqj
eagleoflqj merged commit c2be135 into fcitx-contrib:master Aug 22, 2026
1 check passed
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.

2 participants