-
Notifications
You must be signed in to change notification settings - Fork 45
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
Fix: Set icon for app in windows and mac #596
Conversation
Please fix the title of this PR. Examples:
See more in |
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.
The Windows logo failed on build
Pull request has been marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe changes in this pull request involve updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Builder
User->>App: Launch Application
App->>Builder: Build for macOS/Windows
Builder->>Builder: Set Icon for macOS
Builder->>Builder: Set Icon for Windows
Builder->>App: Build Complete
App->>User: Display Application
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
electron-builder.yml (2)
18-18
: Add newline at end of fileThe file is missing a newline character at the end, which is a POSIX requirement.
win: icon: packages/renderer/src/assets/logos/fishOnlyLogo.ico +
🧰 Tools
🪛 yamllint (1.35.1)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
15-18
: Consider moving icons to buildResources directoryCurrently, the icons are placed in the renderer package's assets directory. Since these are build-time resources, it would be more appropriate to move them to the
buildResources
directory that's already configured in this file.This separation would:
- Keep build assets separate from application code
- Follow electron-builder's conventional structure
- Make it easier to manage build resources
Suggested structure:
buildResources/ icons/ fishLogo.icns # macOS icon fishOnlyLogo.ico # Windows icon
Then update the paths:
mac: - icon: packages/renderer/src/assets/logos/fishLogo.icns + icon: buildResources/icons/fishLogo.icns win: - icon: packages/renderer/src/assets/logos/fishOnlyLogo.ico + icon: buildResources/icons/fishOnlyLogo.ico🧰 Tools
🪛 yamllint (1.35.1)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/renderer/src/assets/logos/fishOnlyLogo.ico
is excluded by!**/*.ico
packages/renderer/src/assets/logos/logoFishAndText.jpg
is excluded by!**/*.jpg
packages/renderer/src/assets/logos/logoFishOnly.png
is excluded by!**/*.png
packages/renderer/src/assets/logos/logoFishOnly.svg
is excluded by!**/*.svg
📒 Files selected for processing (2)
electron-builder.yml
(1 hunks)packages/renderer/src/components/topBar/TopBar.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/renderer/src/components/topBar/TopBar.tsx
🧰 Additional context used
🪛 yamllint (1.35.1)
electron-builder.yml
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
electron-builder.yml (1)
15-18
: Verify icon file paths exist
The icon paths look correct in terms of format (.icns for macOS, .ico for Windows). Let's verify these files exist in the repository.
✅ Verification successful
Icon paths verified successfully
Both icon files exist at the specified locations:
packages/renderer/src/assets/logos/fishLogo.icns
for macOSpackages/renderer/src/assets/logos/fishOnlyLogo.ico
for Windows
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the specified icon files exist in the repository
# Check for macOS icon
if fd "fishLogo.icns" packages/renderer/src/assets/logos/; then
echo "✓ macOS icon found"
else
echo "✗ macOS icon missing"
fi
# Check for Windows icon
if fd "fishOnlyLogo.ico" packages/renderer/src/assets/logos/; then
echo "✓ Windows icon found"
else
echo "✗ Windows icon missing"
fi
Length of output: 311
🧰 Tools
🪛 yamllint (1.35.1)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
electron-builder.yml (1)
20-20
: Add newline at end of fileAdd a newline character at the end of the file to comply with POSIX standards.
win: icon: packages/renderer/src/assets/logos/fishOnlyLogo.ico +
🧰 Tools
🪛 yamllint (1.35.1)
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
electron-builder.yml
(1 hunks)packages/renderer/src/components/topBar/TopBar.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/renderer/src/components/topBar/TopBar.tsx
🧰 Additional context used
🪛 yamllint (1.35.1)
electron-builder.yml
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
electron-builder.yml (1)
17-20
: Verify icon files existence and formats
The icon paths look correct, but let's verify:
- The icon files exist in the specified locations
- The files are in the correct format for their respective platforms
✅ Verification successful
Icon files are present and correctly configured
Both icon files exist at the specified paths and are being used exclusively for their respective platforms. The search also revealed another icon file at buildResources/icon.icns
, but this doesn't conflict with our configuration as we're explicitly setting platform-specific icons.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify icon files existence and basic format validation
# Check if files exist
echo "Checking if icon files exist..."
for icon_path in "packages/renderer/src/assets/logos/fishLogo.icns" "packages/renderer/src/assets/logos/fishOnlyLogo.ico"; do
if [ -f "$icon_path" ]; then
echo "✓ Found: $icon_path"
# Check file type
file "$icon_path"
else
echo "✗ Missing: $icon_path"
fi
done
# Search for any other icon files to ensure we're using the right ones
echo -e "\nListing all icon files in the project..."
fd -e icns -e ico
Length of output: 1253
🧰 Tools
🪛 yamllint (1.35.1)
[error] 20-20: no new line character at the end of file
(new-line-at-end-of-file)
🎉 This PR is included in version 2.5.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Bug Fixes