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

PiGro 24.01 Release Preparation #2490

Closed
actionschnitzel opened this issue Nov 28, 2023 · 7 comments · Fixed by #2492
Closed

PiGro 24.01 Release Preparation #2490

actionschnitzel opened this issue Nov 28, 2023 · 7 comments · Fixed by #2492
Labels
suggestion New feature or request

Comments

@actionschnitzel
Copy link
Contributor

actionschnitzel commented Nov 28, 2023

Hi Pi-Apps Team,

I plan to release PiGro 24.01 in January.
Pi-Apps currently fetches PiGro from GitHub. Whenever I needed to make changes, I submitted a pull request to your repository. However, for the past six months, I have also been providing DEBIAN packages. What is more convenient for you?

This is not a critical issue. I just wanted to let you know in case you need to make major changes that take time.

New Dependencies:

  • git
  • xterm
  • python3-pil
  • python3-pil.imagetk
  • python3-psutil
  • python3-distro
  • python3-bs4
  • python3-dev
  • python3-requests
  • mpg123
  • lolcat
  • wmctrl
  • gdebi
  • mousepad

(Note: Git may not need to be installed via Pi-Apps)

Link to Latest Release:
PiGro Latest Release

DEBIAN Package Download:
As seen in your FreeTub installer:

version=23.04
https://github.com/actionschnitzel/PiGroAid-/releases/download/${version}/pigro_jci-${version%}_all.deb

I would also like to submit updated icons:
icon-24
icon-64

Best regards
Timo

@actionschnitzel actionschnitzel added the suggestion New feature or request label Nov 28, 2023
@Botspot
Copy link
Owner

Botspot commented Nov 28, 2023

Thanks for the heads-up. We're fine with switching to your deb release. Do you have your own apt repo? If not, that's fine.
This looks like a very simple change. Pi-Apps' main strength is its flexibility. It's a script, so any app can install by any means necessary.
While I could go ahead and make these changes myself, it would probably save us both time if you opened a pull request with the proposed changes. and yes, it looks like a copy of the FreeTube scripts would be a great starting point. This also gives you the chance to shoutout the new features in the description. If you can't tell, I think PiGro is pretty awesome especially for intermediate users who want to get stuff done quickly.

@theofficialgman
Copy link
Collaborator

theofficialgman commented Nov 28, 2023

@actionschnitzel I would prefer that you package your deb properly if we are to switch to it.
you should not be using prerm, postrm, and postinst scripts to create/delete a /usr/share/applications/pigro.desktop file or modify permissions of any files that were just installed. Permissions should be corrected in the data archive itself and all files should be present in that. Right now your deb is very hacky for no reason.

The owner of the files is also incorrect in the data archive as well. The files are owned by the user "timo" when they should be "root" owned.

@actionschnitzel
Copy link
Contributor Author

@theofficialgman Wow, good that you've taken a closer look. I am using a script that I found. Must fix this as soon as possible.

@actionschnitzel
Copy link
Contributor Author

Update:
I fixed the ownership and permissions for pigro-jci-23.04.deb and also created a proper directory structure. I hope everything is now as it should be. If not, just let me know.

Thanks to both of you for your help and constructive criticism. It's appreciated.

Once everything is ready, I will create a pull request.

@theofficialgman
Copy link
Collaborator

theofficialgman commented Nov 30, 2023

Update: I fixed the ownership and permissions for pigro-jci-23.04.deb and also created a proper directory structure. I hope everything is now as it should be. If not, just let me know.

Much better.

One thing to note about your .desktop file. You shouldn't need to specify the full path to the icon for it to work
Icon=/usr/share/icons/hicolor/256x256/apps/pigro-logo.png. Just the name of the icon should do Icon=pigro-logo. That lets users who have custom themed icons easily overwrite yours without having to edit the .desktop file manually. Additionally, if you have vectored versions of your logo that is generally a good thing to provide as well, they go under /usr/share/icons/hicolor/scalable/apps as an .svg file.

Yes I realize that this icon only gets used briefly during launch of your application before the main python script tells tkinter to use a specific hardcoded icon but it is still good practice. I will let you play around with the python code and see if you can get away with not specifying the full path and still have the icon load there as well.

@actionschnitzel
Copy link
Contributor Author

actionschnitzel commented Nov 30, 2023

"It's always good to be nudged in the right direction :-) The .svg is now included. I replaced "self.iconphoto(True, self.icon)" (still needs to be completely removed) with "__init__(className="PiGro")". One less thing giving me a headache!

20231130_12h44m19s_grim
20231130_13h40m23s_grim

I've packed v23.04 and a DEV-Preview of v24.01... Just in case there's interest."
PIGRO-DEBS.tar.gz

@theofficialgman
Copy link
Collaborator

theofficialgman commented Nov 30, 2023

"It's always good to be nudged in the right direction :-) The .svg is now included. I replaced "self.iconphoto(True, self.icon)" (still needs to be completely removed) with "__init__(className="PiGro")". One less thing giving me a headache!

Right yeah, regarding the className variable, wayland based compositors actually can't set the window/titlebar icon through the old methods used in X11. What wayland compositors (like wayfire) do to get around this is check the class (and sometimes other environment variables) and see what .desktop files have a matching StartupWMClass or filename. the checks are different depending on the DE (eg: piOS wayfire, gnome, kde) and can be a little fuzzy so what works in one DE might not work in another (eg: some are case sensitive, others are not). I checked your deb and what you did looks good. That should work on everything since matching the set env variable class with the .desktop files StartupWMClass is the most supported case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants