-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added possibility to specify platform for dev command #3248
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe recent updates to the Wails framework focus on refining the build process, particularly around handling multiple targets, platform-specific logic, and error management. Enhancements include simplifying target iteration, improving platform and architecture assignments, and streamlining the initialization and parsing of build flags. Additionally, support for specifying platforms in development mode has been introduced, alongside better detection and handling for Windows Subsystem for Linux (WSL) environments. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (7)
- v2/cmd/wails/build.go (6 hunks)
- v2/cmd/wails/flags/build.go (4 hunks)
- v2/cmd/wails/flags/common.go (1 hunks)
- v2/cmd/wails/flags/dev.go (4 hunks)
- v2/cmd/wails/internal/dev/dev.go (3 hunks)
- v2/pkg/commands/build/build.go (2 hunks)
- website/src/pages/changelog.mdx (1 hunks)
Additional comments: 21
v2/cmd/wails/flags/common.go (5)
- 16-19: The
Target
struct is well-defined for holding platform and architecture information.- 21-39: The
defaultTarget
function correctly determines the default platform and architecture, including special handling for Apple Silicon.- 43-53: The
MacTargetsCount
method inTargetsCollection
correctly counts targets for macOS platforms.- 55-61: The
String
method forTarget
struct correctly formats the target as a string.- 63-79: The
parseTargets
function correctly parses platform strings intoTarget
instances, including default architecture handling.v2/cmd/wails/flags/build.go (2)
- 52-55: The
Default
method inBuild
struct correctly initializes default values, including the platform fromdefaultTarget
.- 74-75: The
GetTargets
method correctly utilizesparseTargets
for parsing the platform field.v2/cmd/wails/flags/dev.go (2)
- 19-19: The addition of the
Platform
field in theDev
struct is correctly annotated with a description.- 41-45: The
Default
method inDev
struct correctly initializes thePlatform
field usingdefaultTarget
.v2/cmd/wails/build.go (7)
- 111-111: The condition for adding the "Output File" to the build options table is correctly updated to check the number of targets.
- 149-152: The loop correctly checks for valid platform and architecture combinations and skips unsupported ones.
- 161-163: Correctly assigns platform and architecture to
buildOptions
from the target.- 167-167: Correctly warns about the unsupported compress flag for universal binaries on macOS.
- 183-183: Correctly sets the bundle name for macOS targets when there are two Mac targets.
- 173-191: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [188-197]
Correctly handles the output filename for multiple targets, including platform-specific adjustments.
- 232-245: The logic for generating NSIS installers for Windows targets is correctly implemented, including error handling for missing targets.
v2/pkg/commands/build/build.go (1)
- 74-76: The
IsWindowsTargetPlatform
method correctly checks if the platform is Windows.v2/cmd/wails/internal/dev/dev.go (3)
- 64-64: The
runCommand
function correctly executes a command with the provided arguments.- 271-279: The
isWsl
function correctly detects if the current environment is WSL by reading/proc/version
.- 319-323: Correctly sets the
WSLENV
environment variable for Windows targets under WSL to pass through specified variables.website/src/pages/changelog.mdx (1)
- 17-18: LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (4)
- v2/cmd/wails/flags/build.go (4 hunks)
- v2/cmd/wails/internal/dev/dev.go (3 hunks)
- v2/pkg/commands/build/build.go (2 hunks)
- website/src/pages/changelog.mdx (1 hunks)
Additional comments: 6
v2/cmd/wails/flags/build.go (2)
- 51-54: The change to use a
target
struct for setting default values in theBuild
struct'sDefault
method is a good practice for maintainability. Ensure thedefaultTarget
function is correctly implemented and covered by tests.- 73-74: The update to return a
TargetsCollection
fromGetTargets
enhances the handling of target platforms. Verify that all usages ofGetTargets
have been updated to accommodate the new return type.v2/pkg/commands/build/build.go (1)
- 74-76: The addition of
IsWindowsTargetPlatform
improves code readability and maintainability by centralizing the platform check logic. Ensure this method is consistently used wherever a Windows platform check is required.v2/cmd/wails/internal/dev/dev.go (2)
- 274-282: The addition of the
isWsl
function is beneficial for detecting WSL environments. Ensure this function is correctly used wherever a WSL check is required.- 322-326: Adjusting environment variables for Windows targets under WSL using
WSLENV
is a good practice. Verify that all necessary environment variables are correctly passed through and handled.website/src/pages/changelog.mdx (1)
- 21-21: The summary mentions the addition of an option to specify the platform for the
dev
command, but it's not clear if this is a command-line flag or a configuration option. Clarifying this in the changelog could help users understand how to use this new feature.
@PylotLight - you still keen to get this in? |
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration using
wails doctor
.Test Configuration
Please paste the output of
wails doctor
. If you are unable to run this command, please describe your environment in as much detail as possible.Checklist:
website/src/pages/changelog.mdx
with details of this PRSummary by CodeRabbit
New Features
Refactor
Documentation