Skip to content

Conversation

@tigerros
Copy link

@tigerros tigerros commented Dec 1, 2023

This finishes the empty publishing chapter. cargo-packager is the tool used, but alternatives are mentioned.

@codecov
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9bf1cf2) 52.47% compared to head (0bfddc0) 53.11%.
Report is 11 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #407      +/-   ##
==========================================
+ Coverage   52.47%   53.11%   +0.63%     
==========================================
  Files         145      147       +2     
  Lines       12847    12989     +142     
==========================================
+ Hits         6742     6899     +157     
+ Misses       6105     6090      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marc2332 marc2332 self-requested a review December 2, 2023 09:45
@marc2332
Copy link
Owner

marc2332 commented Dec 2, 2023

I will soon get back to #390, so maybe you can leave some reference in the docs?

@tigerros
Copy link
Author

tigerros commented Dec 2, 2023

Sure. The link to the example doesn't work now though, because I changed the fork branch to the main branch. So #390 needs to be merged first (unless you're fine with a broken link...).

@marc2332 marc2332 added documentation Improvements or additions to documentation enhancement 🔥 New feature or request labels Dec 3, 2023
@marc2332
Copy link
Owner

It would be nice to also mention winresource in the instructions

the crate lets you apply a custom icon to the executable, I have updated the example with the new winresource usage

https://github.com/marc2332/freya/pull/390/files#diff-814556d4ee71407537d0eebc7e55bb0a378c7948b755d02c0bf2ce62fb89dabf

@tigerros
Copy link
Author

It would be nice to also mention winresource in the instructions

the crate lets you apply a custom icon to the executable, I have updated the example with the new winresource usage

https://github.com/marc2332/freya/pull/390/files#diff-814556d4ee71407537d0eebc7e55bb0a378c7948b755d02c0bf2ce62fb89dabf

Doesn't cargo-packager do this?

@marc2332
Copy link
Owner

It would be nice to also mention winresource in the instructions
the crate lets you apply a custom icon to the executable, I have updated the example with the new winresource usage
https://github.com/marc2332/freya/pull/390/files#diff-814556d4ee71407537d0eebc7e55bb0a378c7948b755d02c0bf2ce62fb89dabf

Doesn't cargo-packager do this?

No, not for Windows. Installer icon != executable icon

@tigerros
Copy link
Author

tigerros commented Dec 18, 2023

It would be nice to also mention winresource in the instructions
the crate lets you apply a custom icon to the executable, I have updated the example with the new winresource usage
https://github.com/marc2332/freya/pull/390/files#diff-814556d4ee71407537d0eebc7e55bb0a378c7948b755d02c0bf2ce62fb89dabf

Doesn't cargo-packager do this?

No, not for Windows. Installer icon != executable icon

Sorry for not replying sooner. I looked at the cargo-packager examples and they don't use winresource, but the executables do have icons (at least the tauri example, which is the most extensive one). I'm on Windows as well.

@marc2332
Copy link
Owner

marc2332 commented Dec 18, 2023

Sorry for not replying sooner

it's fine, I wasn't expecting an answer right away

I looked at the cargo-packager examples and they don't use winresource, but the executables do have icons (at least the tauri example, which is the most extensive one). I'm on Windows as well.

I didn't test the Tauri one, I only tried the Dioxus one. If you install it, the app will not have an icon.

image

You can see the discord discussion around this here. I wasn't the one who came up with using winresource, they told me themself

@tigerros
Copy link
Author

tigerros commented Dec 18, 2023

But...
image

Also what discord server is that?

@marc2332
Copy link
Owner

marc2332 commented Dec 18, 2023

But... image

The Tauri example doesn't apply here as it depends on tauri-build and Tauri's CLI. It is Tauri who is changing the icon, not cargo-packager.

@tigerros
Copy link
Author

But... image

The Tauri example doesn't apply here as it depends on tauri-build and Tauri's CLI. It is Tauri who is changing the icon, not cargo-packager.

That makes sense. I don't mind winresource, I just don't like the fact that it's only for Windows, while Freya and cargo-packager are supposed to be cross-platform. Do you know of a cross-platform solution?

@marc2332
Copy link
Owner

marc2332 commented Dec 18, 2023

Also what discord server is that?

Open the link, it is the official CrabNebula Discord Server, authors of cargo-packager

@marc2332
Copy link
Owner

But... image

The Tauri example doesn't apply here as it depends on tauri-build and Tauri's CLI. It is Tauri who is changing the icon, not cargo-packager.

That makes sense. I don't mind winresource, I just don't like the fact that it's only for Windows, while Freya and cargo-packager are supposed to be cross-platform. Do you know of a cross-platform solution?

The fact that the executable doesn't have an icon, only happens in Windows. Linux and MacOS is already handled by cargo-packager, you can read it in the conversation I had

@tigerros
Copy link
Author

Also what discord server is that?

Open the link, it is the official CrabNebula Discord Server, authors of cargo-packager

It doesn't invite me automatically. Had to look for it on google.

But... image

The Tauri example doesn't apply here as it depends on tauri-build and Tauri's CLI. It is Tauri who is changing the icon, not cargo-packager.

That makes sense. I don't mind winresource, I just don't like the fact that it's only for Windows, while Freya and cargo-packager are supposed to be cross-platform. Do you know of a cross-platform solution?

The fact that the executable doesn't have an icon, only happens in Windows. Linux and MacOS is already handled by cargo-packager, you can read it in the conversation I had

Ok I'll add it. Sorry for being maybe a little too stubborn

@marc2332
Copy link
Owner

Also what discord server is that?

Open the link, it is the official CrabNebula Discord Server, authors of cargo-packager

It doesn't invite me automatically. Had to look for it on google.

But... image

The Tauri example doesn't apply here as it depends on tauri-build and Tauri's CLI. It is Tauri who is changing the icon, not cargo-packager.

That makes sense. I don't mind winresource, I just don't like the fact that it's only for Windows, while Freya and cargo-packager are supposed to be cross-platform. Do you know of a cross-platform solution?

The fact that the executable doesn't have an icon, only happens in Windows. Linux and MacOS is already handled by cargo-packager, you can read it in the conversation I had

Ok I'll add it. Sorry for being maybe a little too stubborn

No worries.

@tigerros
Copy link
Author

Hey, just letting you know that there's nothing else to add. I'm only saying this because you responded to the commits I made and I doubt you've been reviewing it for 11 hours :)

@marc2332
Copy link
Owner

Hey, just letting you know that there's nothing else to add. I'm only saying this because you responded to the commits I made and I doubt you've been reviewing it for 11 hours :)

Don't worry, I add myself as reviewer, so I don't forget about it. I was working until just a few hours ago, so I'll check this asap

@tigerros
Copy link
Author

Don't worry, I add myself as reviewer, so I don't forget about it.

I know I was just confused since you replied with no notes and this is a very small change that barely needs a review. I thought you missed the fact that I made the commits and only saw the message... I didn't realize you could be at work :L

Copy link
Owner

@marc2332 marc2332 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 🔥 🔥 🔥

@marc2332
Copy link
Owner

Nice! Ty!

@marc2332 marc2332 merged commit 4ef5a94 into marc2332:main Dec 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement 🔥 New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants