ionutbortis / gnome-bedtime-mode

Home of the "Bedtime Mode" :zzz: GNOME Shell Extension.
GNU General Public License v3.0
78 stars 12 forks source link

Script fixes #17

Closed daPhipz closed 3 years ago

daPhipz commented 3 years ago

I wanted to contribute a German translation, but as unable to create the po/de.po file with the scripts. So instead, I made the scripts a little bit more robust ;) After debugging a bit by myself, I will maybe open an issue about not being able to create a translation.

ionutbortis commented 3 years ago

Hi Philipp,

Thank you for the shell code fixes! They look fine to me, even though I'm not an expert at writing bash scripts, as you've already seen :sweat_smile:

I find the "cd" extra check a little overkill, tbh. I get the rationale behind it but in our context I don't think it should be needed because you'll get the scripts with the rest of the repo code. I'm usually inclined to write minimal code in order to avoid future head scratchings. I'll leave it there to have it for future reference :grin:

I just generated the de.po file on my machine it seemed to work... I can add it to the repo if you want to use it for the German translation, just let me know about it.

Yeah, it would be nice to have the languages script run on as many machines as possible. As a workaround I could add a README note that guides the translator to use the .pot file as source in their preferred translation program. I think I saw that option when I played around with Gtranslator and POEdit.

Let me know what are your thoughts about this. Thanks!

daPhipz commented 3 years ago

Thanks for the quick response, I appreciate it!

I find the "cd" extra check a little overkill, tbh.

It may seem like overkill at first glance, but this is what actually threw the error for me (together with not putting variables in double quotes): My repos for GNOME Extensions I contribute to are in a folder ~/Programming/GNOME Extensions - the script used to cut off the path to the repo like so: ~/Programming/GNOME since it thought that after the space character, the path has ended. That's why it is needed. If we would not catch the error, the script would go on and try to execute all commands as if nothing happened - which is not what we want, of course ;)

As a side note, since you say you are no expert at bash scripting: Would you be interested to switch the build system to use make? IMO, this would streamline the build process and (since make is widely known) make it easier to understand for new contributors.

Let me know what you think, I'd be happy to file a PR!

ionutbortis commented 3 years ago

Oh, spaces in paths, cool, not :sweat_smile:

I actually considered 'make' but then I remembered it's another tool I need to revisit since my golden days when I worked for HP. I thought shell scripts are more popular than make and it will be easier for anyone to just run a script file.

I'll have to read some articles on why a switch to make or something else would be better. Honestly, I'm not expecting the existing scripts to change much over time, so maybe I'll choose the easy path and leave them as they are :sweat_smile: