-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow to specify macos-min-version externally #3756
Allow to specify macos-min-version externally #3756
Conversation
WalkthroughThe changes made to the Changes
Possibly related PRs
Suggested reviewers
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration 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.
Could you please update the documentation? Thank you 🙏
@leaanthony missed your comment, sorry for delay. I added note in documentation for this |
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
website/docs/reference/cli.mdx (1)
106-111
: Approve addition with minor grammatical correctionsThe new info box providing information on setting a minimal version for macOS is a valuable addition to the documentation. It offers clear instructions on how to override the default minimal version, which aligns well with the PR objectives.
However, there are two minor grammatical issues to address:
- "MacOS" should be "macOS" for consistency with Apple's branding.
- Consider adding "the" before "default" in the second sentence for improved readability.
Here's a suggested revision for the content:
:::info Set minimal version for macOS You can override the default [minimal version](../gettingstarted/installation#supported-platforms) of macOS for your app by providing version via `CGO_CFLAGS` and `CGO_LDFLAGS` environment variables. e.g. `CGO_CFLAGS=-mmacosx-version-min=10.15.0 CGO_LDFLAGS=-mmacosx-version-min=10.15.0 wails build` :::🧰 Tools
🪛 LanguageTool
[grammar] ~106-~106: The operating system from Apple is written “macOS”.
Context: .... ::: :::info Set minimal version for MacOS You can override default [minimal vers...(MAC_OS)
[uncategorized] ~108-~108: You might be missing the article “the” here.
Context: ...mal version for MacOS You can override default [minimal version](../gettingstarted/ins...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- website/docs/reference/cli.mdx (1 hunks)
- website/src/pages/changelog.mdx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/src/pages/changelog.mdx
🧰 Additional context used
🪛 LanguageTool
website/docs/reference/cli.mdx
[grammar] ~106-~106: The operating system from Apple is written “macOS”.
Context: .... ::: :::info Set minimal version for MacOS You can override default [minimal vers...(MAC_OS)
[uncategorized] ~108-~108: You might be missing the article “the” here.
Context: ...mal version for MacOS You can override default [minimal version](../gettingstarted/ins...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
🔇 Additional comments (2)
website/docs/reference/cli.mdx (2)
Line range hint
112-116
: Approve addition of UPX on Apple Silicon informationThe new info box about UPX issues on Apple Silicon is a valuable addition to the documentation. It provides important information for developers using Apple Silicon devices and includes a link to the relevant GitHub issue for those seeking more details.
This addition aligns well with the PR objectives by enhancing the documentation with platform-specific considerations.
🧰 Tools
🪛 LanguageTool
[grammar] ~106-~106: The operating system from Apple is written “macOS”.
Context: .... ::: :::info Set minimal version for MacOS You can override default [minimal vers...(MAC_OS)
[uncategorized] ~108-~108: You might be missing the article “the” here.
Context: ...mal version for MacOS You can override default [minimal version](../gettingstarted/ins...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
Line range hint
106-116
: Overall assessment: Valuable additions to the CLI documentationThe changes made to this file enhance the documentation by providing important information for macOS developers. The additions about setting a minimal macOS version and UPX issues on Apple Silicon are well-placed and relevant.
These changes align perfectly with the PR objectives of allowing developers to specify a higher minimum macOS version and improving the overall documentation. The new content will be beneficial for users working with macOS and Apple Silicon devices.
🧰 Tools
🪛 LanguageTool
[grammar] ~106-~106: The operating system from Apple is written “macOS”.
Context: .... ::: :::info Set minimal version for MacOS You can override default [minimal vers...(MAC_OS)
[uncategorized] ~108-~108: You might be missing the article “the” here.
Context: ...mal version for MacOS You can override default [minimal version](../gettingstarted/ins...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
@leaanthony not sure why, but some mandatory checks for tests don't run. Should them been triggered somehow? |
Quality Gate passedIssues Measures |
Thanks again @APshenkin 🙏 |
Description
Wails setup
mmacosx-version-min
while building the app. However there could be the cases, when developers want to specify higher min version. Currently it's not possible, cause latestmmacosx-version-min
is taken as priorityFixes # (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
ZoomFactor
andIsZoomControlEnabled
options are now Windows-only.