Comment 2 for bug 1762889

Revision history for this message
Iain Lane (laney) wrote :

Thanks for the patch. I'm of the opinion that this isn't bionic-critical, since these are packages which have never worked with dh_translations + meson, do you agree?

Here's my review:

+ # known domains to be ignored in this context
+ my %ignores = (
+ 'gnome-control-center-2.0-timezones' => 1,
+ 'libgweather-locations' => 1,
+ );

I don't like this - it puts the information in a hard to find place that's remote from the packages it refers to. I think this should be in the rules file for the affected projects, maybe

  $ dh_translations --ignore-domain=gnome-control-center-2.0-timezones

and documented in the manpage. Ideally you'd be able to specify that option multiple times.

+ # delete if found in %ignores or begins with 'help-'
+ for my $d (keys %domains) {
+ delete $domains{$d} if $ignores{$d} or $d =~ /^help-/;
         }

I feel like you could end up accidentally ignoring too many things here. The documentation says

  This also creates two targets for translations help-$project-update-po and help-$project-pot.

So AFAIK we don't need to guess at the domain to ignore since you can do something like

  $ meson introspect . --projectinfo | jq -r '.name'

to get $project.

+ ($domain) = keys %domains;

I'm not a great perl expert, but is this a normal idiom? I guess it gets the first element - is it preferable to using [0]? Just reads a bit confusing to me but if it's normal perl then that's OK.

I've attached a perl file that shows how to get the difference of two arrays with only one map and getting an array out at the end - it might be useful for this bug.

Thanks again!