Skip to content
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

#3200: Use OutputFilename instead of Name #3551

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

plutov
Copy link

@plutov plutov commented Jun 18, 2024

Issue: #3200

Solution: use options.ProjectData.OutputFilename instead of options.ProjectData.Name

Summary by CodeRabbit

  • Bug Fixes

    • Improved error messaging in the MacOS build process to reference "CompiledBinary" instead of "OutputFilename".
    • Fixed the MacOS build process to correctly utilize outputfilename from wails.json.
  • Documentation

    • Updated the changelog with the latest fix for the MacOS build process.

Copy link
Contributor

coderabbitai bot commented Jun 18, 2024

Walkthrough

The recent updates improve clarity and accuracy in code, especially in error messages, during the MacOS build process. By utilizing fields from the wails.json configuration file, the MacOS build now handles filenames more consistently, specifically changing error references to reflect CompiledBinary instead of OutputFilename. Additionally, the change is documented in the unreleased version's changelog for increased transparency.

Changes

File Summary
v2/pkg/commands/build/packager.go Updated error message to reference CompiledBinary instead of OutputFilename.
website/src/pages/changelog.mdx Documented the fix in the changelog regarding the use of outputfilename from wails.json.

Poem

In the land of code where binaries compile,
A rabbit hops, updating with style. 🐇
With clearer messages for Mac builds bright,
Now CompiledBinary takes the spotlight! ✨
In changelogs, these tweaks reside,
Better builds on a smoother ride. 🚀


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?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5cd0cac and 349c115.

Files selected for processing (1)
  • v2/pkg/commands/build/packager.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • v2/pkg/commands/build/packager.go

@leaanthony
Copy link
Member

Thanks 🙏 Please could you add an entry to the changelog located at website/src/pages/changelog.mdx?

@leaanthony
Copy link
Member

What happens if you set your output filename in the Json to a blank string?

@plutov
Copy link
Author

plutov commented Jun 19, 2024

Good question @leaanthony I noticed that we set it to name by default:

if p.OutputFilename == "" {
  p.OutputFilename = p.Name
}

@leaanthony
Copy link
Member

LGTM. If you could just update the change log we'll look to get this in

@plutov
Copy link
Author

plutov commented Jun 19, 2024

@leaanthony done, I put it under Unreleased, not sure if it's the right place.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 349c115 and cf070d9.

Files selected for processing (1)
  • website/src/pages/changelog.mdx (1 hunks)
Additional context used
LanguageTool
website/src/pages/changelog.mdx

[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ... existing functionality. - Deprecated for soon-to-be removed features. - `Removed...


[typographical] ~11-~11: Consider adding a comma here. (FOR_NOW_COMMA)
Context: ...oon-to-be removed features. - Removed for now removed features. - Fixed for any bug...


[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...edfor now removed features. -Fixedfor any bug fixes. -Security` in case of ...


[duplication] ~17-~17: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ... vulnerabilities. ## [Unreleased] ### Fixed - Fixed MacOS build to use outputfilename fro...


[grammar] ~18-~18: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...es. ## [Unreleased] ### Fixed - Fixed MacOS build to use outputfilename from wail...


[duplication] ~22-~22: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...sues/3200) ## v2.9.1 - 2024-06-18 ### Fixed - Fixed build issues on Linux for some glibc ve...


[duplication] ~27-~27: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...eaanthony. ## v2.9.0 - 2024-06-16 ### Added - Added Drag & Drop (files or folders) support ...


[grammar] ~43-~43: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...etPosition` values were inconsistent on MacOS. Fixed by [@cenan](https://github.com/w...


[duplication] ~52-~52: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...b.com//issues/3421). ### Added - Added support for proxying assets requests to...


[duplication] ~55-~55: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...thub.com//pull/3463) ### Fixed - Fixed an issue with missing icon for Windows....


[style] ~62-~62: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...u can choose between vscode and goland. Added by @DiegPS in [PR](https://github.com/w...


[style] ~69-~69: Consider using a different verb for a more formal wording. (FIX_RESOLVE)
Context: .../pull/3289) ### Fixed - Fixed an issue where embed directives with pa...


[style] ~77-~77: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...the default name to the project’s name. Changed by [@Twacqwq](https://github.com/Twacqw...


[style] ~83-~83: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...ports DisablePinchZoom configuration. Added by @tuuzed in [PR](https://github.com/w...


[duplication] ~90-~90: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...thub.com//pull/3157) ### Fixed - Fixed vue-ts template build error. Fixed by @...


[uncategorized] ~96-~96: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ails/pull/3154)). - Fixed React Routing documentation add 'Routes' and 'Route'. Fixed by @dOr...


[uncategorized] ~157-~157: “its” (belonging to it) seems less likely than “it’s” (it is) (AI_HYDRA_LEO_CPT_ITS_ITIS)
Context: ...tion and wrong initial window size when its enabled. Added by @lyimmi in [PR](https...


[uncategorized] ~159-~159: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Context: ...Fixed filesystem watcher from filtering top level project directory if binary name is inc...


[uncategorized] ~165-~165: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: .../pull/2876) ### Added - Added correct NodeJS and Docker package names for DNF packag...


[style] ~165-~165: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...s for DNF package manager of Fedora 38. Added by @aranggitoar in [PR](https://github....


[style] ~170-~170: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ... frontend. - Added sveltekit.mdx guide. Added by @figuerom16 in [PR](https://github.c...


[style] ~195-~195: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...ring fast reloads of requests on macOS. Fixed by @stffabi in [PR](https://github.com/...


[uncategorized] ~207-~207: The official spelling of this programming framework is “Node.js”. (NODE_JS)
Context: ...pp/wails/pull/2610) ### Added - Added Nodejs version in wails doctor. Added by @mi...


[style] ~207-~207: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...Added Nodejs version in wails doctor. Added by @misitebao in [PR](https://github.co...


[grammar] ~208-~208: The singular determiner ‘this’ may not agree with the plural noun ‘features’. Did you mean “these”? (THIS_NNS)
Context: ...ag webkit2_40 to activate support for this features. This also bumps the minimum r...


[uncategorized] ~209-~209: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Context: ...2) - macOS: Added Window menu role with well known shortcuts "Minimize, Full-Screen and Zo...


[style] ~220-~220: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ... printing in wails generate template. Fixed by @misitebao in [PR](https://github.co...


[style] ~237-~237: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...AssetServer tried to log to the logger. Fixed by @stffabi in [PR](https://github.com/...


[style] ~244-~244: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...cs/reference/options#webviewgpupolicy). Added by @lyimmi in [PR](https://github.com/w...


[grammar] ~245-~245: The word “opt-in” is a noun. The verb is spelled with a white space. (NOUN_VERB_CONFUSION)
Context: ...leFraudulentWebsiteDetection` option to opt-in to scan services for fraudulent content...


[style] ~253-~253: This phrase is redundant. Consider using “outside”. (OUTSIDE_OF)
Context: ...therwise resizing cursor won't be shown outside of the body rectangle. Changed by @stffabi...


[style] ~259-~259: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...ild hooks when build/bin was missing. Fixed by @lyimmi in [PR](https://github.com/w...


[grammar] ~266-~266: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...m//pull/2397) - Fixed the macos single architecture builds not respecti...


[style] ~276-~276: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym. (ENGLISH_WORD_REPEAT_BEGINNING_RULE)
Context: ...artup of the application in debug mode. Added by @stffabi in [PR](https://github.com/...


[grammar] ~282-~282: The singular determiner ‘this’ may not agree with the plural noun ‘features’. Did you mean “these”? (THIS_NNS)
Context: ...ag webkit2_36 to activate support for this features. This also bumps the minimum r...


[grammar] ~399-~399: It appears that an article is missing. (IN_WEBSITE)
Context: ...pp/wails/pull/1965 - Use swc + pnpm for website - @leaanthony in https://github.com/wai...


[grammar] ~456-~456: It appears that an article is missing. (IN_WEBSITE)
Context: ...sapp/wails/pull/1795 - fix: fix bugs in website by @misitebao in https://github.com/wai...


[typographical] ~496-~496: Use a comma after an introductory phrase. (COMMA_INTRODUCTORY_WORDS_PHRASES)
Context: ...o-drag"` to disable the drag behaviour. For this release only, you can test this by sett...


[grammar] ~546-~546: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...pp/wails/pull/1662 - Multiple fixes for MacOS asset requests by @stffabi in https://g...


[grammar] ~551-~551: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...ub.com//pull/1704 - Allow MacOS frameless window to be miniturisable by...


[grammar] ~556-~556: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...1671 - wails doctor now reports correct MacOS os id by @stffabi in https://github.com...


[grammar] ~577-~577: Before the countable noun ‘of’ an article or a possessive pronoun is necessary. (IN_NN_CC_VBG)
Context: ...ny ### Fixed - Fixed initial build of frontend when using wails dev on new projects ...


[uncategorized] ~588-~588: Did you mean “too”? (TOO_JJ_TO)
Context: ...-24 ### Added - Add Show() and Hide() to runtime to show/hide application by @le...


[grammar] ~640-~640: The correct preposition appears to be “on”. (IN_WINDOWS)
Context: ...1555 ### Fixed - Fix stack corruption in Windows when using ICoreWebView2HttpHea...


[grammar] ~753-~753: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...thub.com//pull/1414 - Add macos custom menu EditMenu tips by @daodao97 ...


[grammar] ~816-~816: This expression is usually spelled with a hyphen. (DOUBLE_CLICK_HYPHEN)
Context: ... @napalu - Double Click event now works on elements with `data-...


[grammar] ~838-~838: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...](https://github.com/napalu) in #1249 - MacOS - Show extension by default by [@leaant...


[misspelling] ~842-~842: This word is normally spelled with a hyphen. (EN_COMPOUNDS_CROSS_PLATFORM)
Context: ...tor works only for some directives in a cross platform way by [@stffabi](https://github.com/...


[grammar] ~845-~845: It appears that an article is missing. (IN_WEBSITE)
Context: ...adalessa) in #123 - Use local search on website by [@leaanthony](https://github.com/lea...


[misspelling] ~853-~853: This word is normally spelled with a hyphen. (EN_COMPOUNDS_SELF_STARTING)
Context: ...ilt) in #1247 - [v2] Use os.Args[0] for self starting wails by [@stffabi](https://github.com/...

Additional comments not posted (1)
website/src/pages/changelog.mdx (1)

17-19: The changelog entry under "Unreleased" correctly documents the macOS build configuration change to use outputfilename from wails.json. However, consider expanding this entry to include information about how this change impacts macOS builds, especially any potential differences in behavior or compatibility issues that users should be aware of.

Tools
LanguageTool

[duplication] ~17-~17: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ... vulnerabilities. ## [Unreleased] ### Fixed - Fixed MacOS build to use outputfilename fro...


[grammar] ~18-~18: The operating system from Apple is written “macOS”. (MAC_OS)
Context: ...es. ## [Unreleased] ### Fixed - Fixed MacOS build to use outputfilename from wail...

Copy link

sonarcloud bot commented Jun 19, 2024

@leaanthony
Copy link
Member

I suspect this might be a breaking change for some people 🤔

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.

None yet

2 participants