xsensors icon indicating copy to clipboard operation
xsensors copied to clipboard

Create build script, update icons, update .desktop file, bump rev.

Open Alex313031 opened this issue 2 years ago • 3 comments

I use xsensors on the daily, and use this to get an updated version in i.e. Ubuntu (which comes with 0.70).

Added 16, 24, and 32 icon sizes, updated the .desktop file to point to the correct location and added keywords.

Created a build script that sets optimization flags, and prompts the user for --help, --clean, --gtk2, or --gtk3. (Shows help, runs make clean, builds against GTK2, or builds against GTK3, respectively)

Alex313031 avatar Jan 17 '23 07:01 Alex313031

Thanks for the patchset, much appreciated, see feedback below.

1 . Just a minor complaint, can you update the commit descriptions to be a bit more descriptive? At risk of coming off too critical, commit messages with "Update configure" or "Update Makefile.in" aren't too helpful to others looking through the history. If you need help with improving commit messages, there are many guides online to good git commit practices.

  1. I noticed that you updated "xdg/xsensors.desktop" to point to "/usr/local/bin" instead of /usr/bin. I understand the rational for this change, as /usr/local/bin is used by default, but it would be better to update the makefile logic to template in "@bindir@" for this path. This way whatever path is provided to configure will correctly added to the desktop file. E.g. when distros pick this up, they will be calling "./configure --bindir /usr/bin", but a user might want to use the default /usr/local/bin or even ~/.local/bin (where ~ is the home directory)

  2. While I'm sure the build.sh script seems useful, I'd prefer to update the README to better explain how to build, clean, etc. To me it seems like an unnecessary wrapper.

If you want to update your patchset, please let me know. If not, I can cherry-pick/rework the relevant patches and close this.

Mystro256 avatar Mar 27 '23 23:03 Mystro256

@Mystro256 Sorry for the shitty commit message lol I understand.

Yeah a bindir variable would be better. As far as the icon changes, do you agree?

I dont wanna remove my build script. I have made a similar one for my forks of htop, nvtop, this repo, and geany. I frequently install linux distros on different machines and VMs, and it makes building and installing really quick for me.

SOOO, I would prefer if you cherry pick. Later, I could update the readme with better instructions and make another pull request.

Alex313031 avatar Mar 29 '23 10:03 Alex313031

@Mystro256 Updated my pull request

  • Updated copyright, added my name to the about page

  • Added Escape keyboard shortcut to exit. I would like to make it Ctrl + Q like many other gtk apps, but could figure out how to make it eval two simultaneous keypresses.

  • Updated desktop to just use xsensors for the exec line, as this works no matter where it installed, negating the need for the @bindir@ logic you mentioned.

  • I left the buildscript, because we could still update the readme on building steps. The build script is just an easy way to build or clean the repo.

Alex313031 avatar May 08 '23 19:05 Alex313031