search

artemanufrij / playmymusic

An elementary OS app
http://anufrij.org/melody
95 stars 26 forks source link

Reinstall & Update translations files + French translations + Fixes #172

Closed NathanBnm closed 4 years ago

NathanBnm commented 5 years ago

Hi! This PR comes in replacement of #165. I installed translations to make all the languages available to translation, as many other apps does. I also separated the extra translations files (desktop and appdata and files) into the po/extra folder to make translation easier :smile:

Then I made the existing translations available too (I added them back, and removed fuzzy translations).

Note : I added some tooltips, on the loading spinner, the light / dark background switch and the menu icon, and made those translatable.

I tried to build the app and this is working well. Here is the full changelog:

Changelog

Notice

Don't forget to add a new release before to publish the update :smile:

This PR is ready for review

NathanBnm commented 5 years ago

Idk why the build is falling. It'es working on my computer. I have the same issue here: https://github.com/geigi/cozy/pull/153. Maybe you'll have an idea.

ryonakano commented 5 years ago

@NathanBnm I've also faced this issue that the CI fails when I created a PR in Djaler/Formatter before. At first I did also have no idea what's wrong.

For situations like that, it's a good custom to have a look at the log. Let's see this line:

meson.build:44:6: ERROR:  Script or command 'meson/post_install.py' not found or not executable

It says the script does not found or not executable. It's obvious that it is exist at meson/post_install.py. So the solution is to make sure it is executable.

Open the terminal on your local, cd to the root directory of this repository, and execute the following command:

chmod u+x ./meson/post_install.py

Then commit the change:

git add meson/post_install.py
git commit -m "Make post_install.py executable"
git push origin update-translations

I hope this will work.

ryonakano commented 5 years ago

Also, it's a good custom to keep your PR as possible. Developers often review PRs before merging―what changes did this author commit? Is there failures? The more changes committed, the more time needs to review. So if developers are busy, they may procrastinate to review PRs which have many changes.

This PR has 411 file changes, which has many changes. Updating translation files gets the number of changed files larger, so it can't be helped. But I think separating non-translation changes from the translation related PR should keep both PRs simple. I think we can separate this PR to the following 4 PRs, for example:

PR 1. Add .vscode folder to .gitignore PR 2. Add missing tooltips PR 3. Use … instead of ... PR 4. Translation related PR (e.g. The use of "extra" domain for metadata files, updating translation files, updating French translations, and so on)

I don't force you to do this. I left just as a note. :wink:

NathanBnm commented 5 years ago

I hope this will work.

Thanks a lot! :smile: It had been so obvious that I hadn't even thought about it, that was the issue, I fix it wherever I made the mistake.

NathanBnm commented 5 years ago

I don't force you to do this. I left just as a note. wink

I understand what you mean, but most of the changes I have made are related to translations, and I will be limited to making the following changes, sometimes based on previous changes (for example, adding translations for added items).

I try as much as possible to separate into different commits in order to be able to consult the evolution of the changes :+1: . And most of the changes (in quantity) are automatically generated translations files, there is no particular need to check them all.

Concerning the addition of the .vscode folder to .gitignore, this has no impact on the code, it doesn't really matter.

I'll try to be more careful with that now, thanks for the advice :wink:.I've already done a little better on my last pull request