Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAndroid binding generation now propagates ChangesAndroid binding context
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AndroidBuild
participant CommonFrontend
participant GenerateBindings
participant GoLoader
AndroidBuild->>CommonFrontend: set GOOS=android and CGO_ENABLED=0
CommonFrontend->>GenerateBindings: forward binding context
GenerateBindings->>GoLoader: load Android packages without CGO
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies" 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
v3/internal/commands/android_taskfile_test.go (1)
55-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the removed task and rendered environment contract.
This test checks only the internal Taskfile’s forwarding strings. It would pass while the checked-in Android example still references the deleted
generate:android:bindingstask, and it does not catchCGO_ENABLEDrendering as empty. Add assertions for the example Taskfiles and render/execute the relevant task variables.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@v3/internal/commands/android_taskfile_test.go` around lines 55 - 63, Extend TestAndroidTaskfileBuildUsesAndroidBindingContext to inspect the checked-in Android example Taskfiles and assert they no longer reference the removed generate:android:bindings task. Render or execute the relevant task variables and assert the resulting environment includes a non-empty CGO_ENABLED value, while preserving the existing forwarding and Android binding context assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/examples/android/build/android/Taskfile.yml`:
- Around line 45-46: Remove the obsolete generate:android:bindings dependency
from both Android build task definitions:
v3/examples/android/build/android/Taskfile.yml lines 45-46 and
v3/examples/android/build/Taskfile.yml lines 39-42. Ensure each task retains
common:build:frontend as the only binding path, or references another existing
task where required.
In `@v3/internal/commands/build_assets/android/Taskfile.yml`:
- Around line 49-50: Quote the CGO_ENABLED value as the string "0" in the shared
Android Taskfile values at
v3/internal/commands/build_assets/android/Taskfile.yml:49-50,
v3/examples/android/build/android/Taskfile.yml:45-46, and
v3/examples/mobile/build/android/Taskfile.yml:45-46. Preserve the existing GOOS:
android setting so the {{.CGO_ENABLED | default "0"}} template path receives an
explicit zero and selects the no-CGO behavior.
In `@v3/internal/commands/build_assets/Taskfile.tmpl.yml`:
- Around line 95-98: Update the labels/checksums for build:frontend and
generate:bindings to include GOOS and CGO_ENABLED as platform context, ensuring
changes to either variable invalidate the shared dependencies. Apply this in
v3/internal/commands/build_assets/Taskfile.tmpl.yml,
v3/examples/android/build/Taskfile.yml lines 39-42, and
v3/examples/mobile/build/Taskfile.yml lines 39-42; preserve the existing
dependency calls and variable references.
---
Nitpick comments:
In `@v3/internal/commands/android_taskfile_test.go`:
- Around line 55-63: Extend TestAndroidTaskfileBuildUsesAndroidBindingContext to
inspect the checked-in Android example Taskfiles and assert they no longer
reference the removed generate:android:bindings task. Render or execute the
relevant task variables and assert the resulting environment includes a
non-empty CGO_ENABLED value, while preserving the existing forwarding and
Android binding context assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fe78a5ae-1578-4b60-a72f-dc2146e61c20
📒 Files selected for processing (9)
v3/examples/android/build/Taskfile.ymlv3/examples/android/build/android/Taskfile.ymlv3/examples/mobile/build/Taskfile.ymlv3/examples/mobile/build/android/Taskfile.ymlv3/internal/commands/android_taskfile_test.gov3/internal/commands/build_assets/Taskfile.tmpl.ymlv3/internal/commands/build_assets/android/Taskfile.ymlv3/internal/generator/load_android_test.gov3/pkg/application/application_android_nocgo.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@v3/internal/commands/android_taskfile_test.go`:
- Around line 97-122: The TestAndroidExamplesUseCommonBindingContext test
currently checks frontend task settings only by substring presence, so Android
GOOS configuration can be missing or attached to the wrong task. Parse each
example’s build task and inspect its common:build:frontend dependency, asserting
that both GOOS is "android" and CGO_ENABLED is "0"; retain the existing checks
for the root build task variables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c6e09c6-75f1-4214-b18f-7ea23fc934ef
📒 Files selected for processing (7)
v3/examples/android/build/Taskfile.ymlv3/examples/android/build/android/Taskfile.ymlv3/examples/mobile/build/Taskfile.ymlv3/examples/mobile/build/android/Taskfile.ymlv3/internal/commands/android_taskfile_test.gov3/internal/commands/build_assets/Taskfile.tmpl.ymlv3/internal/commands/build_assets/android/Taskfile.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- v3/internal/commands/build_assets/Taskfile.tmpl.yml
- v3/internal/commands/build_assets/android/Taskfile.yml
- v3/examples/mobile/build/android/Taskfile.yml
- v3/examples/android/build/android/Taskfile.yml
leaanthony
added
android
ChatGPT Codex Connector
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.
Description
Android builds passed the
androidbuild tag to the common binding generatorwithout changing its Go target from the host OS. On macOS, package loading
therefore selected both Darwin files and explicitly tagged Android files,
producing 98 analyzer warnings even though the later Android compilation
succeeded.
This change:
GOOSandCGO_ENABLEDthrough the common frontend/binding task;GOOS=androidandCGO_ENABLED=0, so thisstep still does not require the NDK;
context;
Fixes #5810
Type of change
How Has This Been Tested?
Before the fix, the stock
v3/examples/mobileproject emitted 98 warnings:GOWORK=off wails3 generate bindings -dry -clean=false -f "-tags android,debug"With the fixed CLI and Android task context, the same example processed 229
packages with no analyzer warnings:
GOWORK=off GOOS=android CGO_ENABLED=0 \ wails3 generate bindings -dry -clean=false -f "-tags android,debug"The complete Android task path also succeeds without binding warnings:
Regression and full test suites:
Desktop binding generation was also checked with the default host context and
completed without warnings.
Test Configuration
Checklist:
website/src/pages/changelog.mdxwith details of this PR (not applicable; this is a v3 change and v3 changelog entries are added automatically)Summary by CodeRabbit
GOOS) andCGO_ENABLEDsettings into frontend builds and binding generation.GOOS/CGO_ENABLEDforwarding and that Android no-CGO package loading succeeds.