After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 793042 - Use upstream gettext instead of intltool
Use upstream gettext instead of intltool
Status: RESOLVED FIXED
Product: five-or-more
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: five-or-more-maint
five-or-more-maint
Depends on:
Blocks: 763587
 
 
Reported: 2018-01-30 22:10 UTC by Robert Roth
Modified: 2018-03-15 01:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Migrate to Gettext (7.64 KB, patch)
2018-02-24 16:53 UTC, Ruxandra Simion
none Details | Review
Migrate to Gettext (8.36 KB, patch)
2018-02-24 18:05 UTC, Ruxandra Simion
none Details | Review
Migrate to Gettext (17.17 KB, patch)
2018-02-25 18:15 UTC, Ruxandra Simion
none Details | Review
Migrate to Gettext (17.17 KB, patch)
2018-02-25 18:54 UTC, Ruxandra Simion
committed Details | Review

Description Robert Roth 2018-01-30 22:10:37 UTC
As part of https://wiki.gnome.org/Initiatives/GnomeGoals/GettextMigration, get rid of intltool, use gettext.
Comment 1 Ruxandra Simion 2018-02-24 16:53:35 UTC
Created attachment 368878 [details] [review]
Migrate to Gettext

This is my attempt at migrating to Gettext. I have mainly followed the linked guidelines. It seems to work, but I might have missed something. Let me know if I need to update it somehow. Thanks!
Comment 2 Michael Catanzaro 2018-02-24 17:00:08 UTC
Review of attachment 368878 [details] [review]:

Thanks!

Patch almost looks good to me, except for two issues:

::: data/Makefile.am
@@ +27,3 @@
 appstream_in_files = five-or-more.appdata.xml.in
 appstream_XML = $(appstream_in_files:.xml.in=.xml)
 @APPSTREAM_XML_RULES@

You're going to need to modify the appdata.in file as well, to remove the underscores. To catch errors like this, please run 'make distcheck' and verify that it passes.

::: git.mk
@@ -205,3 @@
 				po/Rules-quot \
 				po/stamp-it \
-				po/.intltool-merge-cache \

Please don't manually modify git.mk, just grab the latest upstream version from GitHub.
Comment 3 Michael Catanzaro 2018-02-24 17:02:10 UTC
(In reply to Michael Catanzaro from comment #2)
> Please don't manually modify git.mk, just grab the latest upstream version
> from GitHub.

(using the link at the top of the file)
Comment 4 Ruxandra Simion 2018-02-24 18:05:36 UTC
Created attachment 368883 [details] [review]
Migrate to Gettext

I made the changes, but I didn't add the git.mk file anymore, considering it needs to be downloaded, and not edited, so it doesn't seem relevant for this patch. I have also run distcheck and it seems to be ok now.
Comment 5 Michael Catanzaro 2018-02-24 21:02:09 UTC
(In reply to Ruxandra Simion from comment #4)
> Created attachment 368883 [details] [review] [review]
> Migrate to Gettext
> 
> I made the changes, but I didn't add the git.mk file anymore, considering it
> needs to be downloaded, and not edited, so it doesn't seem relevant for this
> patch. I have also run distcheck and it seems to be ok now.

'git status' should not show generated files after running ./autogen.sh and make. I think you'll need to update git.mk to ensure that. See https://github.com/behdad/git.mk/pull/33.
Comment 6 Robert Roth 2018-02-25 11:25:56 UTC
Review of attachment 368883 [details] [review]:

Thanks, great job. Looks fine overall, beside adding the latest git.mk, I have a couple more suggestions (only minor stuff).

::: data/five-or-more.desktop.in
@@ +2,3 @@
+Name=Five or More
+Comment=Remove colored balls from the board by forming lines
+Keywords=game;strategy;logic;

Add the following translator comment before the Keywords line:
# Translators: Search terms to find this application. Do NOT translate or localize the semicolons! The list MUST also end with a semicolon!

@@ +5,2 @@
 Exec=five-or-more
 Icon=five-or-more

Before Icon we should also have a translator comment, like the following one:
# Translators: Do NOT translate or transliterate this text (this is an icon file name)!

::: po/Makevars
@@ +20,3 @@
+# their copyright.
+COPYRIGHT_HOLDER = Free Software Foundation, Inc.
+

Copyright holder should be "five-or-more contributors", not Free Software Foundation.

@@ +26,3 @@
+# detect it automatically by scanning the files in $(top_srcdir) for
+# "GNU packagename" string.
+PACKAGE_GNU =

It should be simply "no", five-or-more is not a GNU package.

@@ +42,3 @@
+# can write to without being subscribed, or the URL of a web page through
+# which the translators can contact you.
+MSGID_BUGS_ADDRESS =

Use the following address: "https://bugzilla.gnome.org/enter_bug.cgi?product=five-or-more&keywords=I18N+L10N&component=general"
Comment 7 Ruxandra Simion 2018-02-25 18:15:07 UTC
Created attachment 368901 [details] [review]
Migrate to Gettext

I made the changes.
Comment 8 Piotr Drąg 2018-02-25 18:39:15 UTC
Review of attachment 368901 [details] [review]:

::: po/Makevars
@@ +52,3 @@
+# package uses functions taking also a message context, like pgettext(), or
+# if in $(XGETTEXT_OPTIONS) you define keywords with a context argument.
+USE_MSGCTXT = no

five-or-more does use msgctxt, so this should be set to yes.
Comment 9 Ruxandra Simion 2018-02-25 18:54:02 UTC
Created attachment 368903 [details] [review]
Migrate to Gettext
Comment 10 Robert Roth 2018-02-26 16:27:14 UTC
Review of attachment 368903 [details] [review]:

Looks fine for me, thank you for your patience and the several iterations you have dine. All comments from all three reviewers are properly addressed in this patch, so I am marking as accepted_commit_after_freeze, meaning that this will be applied to master after the 3.28.0 tarball is released and gnome-3-28 branched on 12th March (two weeks from now).
Comment 11 Robert Roth 2018-03-15 01:09:29 UTC
Attachment 368903 [details] pushed as 20399a6 - Migrate to Gettext