Re: [PATCH 4/7] remote-mediawiki: skip virtual namespaces
- From
Antoine Beaupré <anarcat@debian.org>
- Date
- Nov 1, 2017, 16:45 UTC
- Message-ID
- <87wp3a54nj.fsf@curie.anarc.at>
- In-Reply-To
- <CAPig+cSRrXj9BdgM6Q2OdBZtYJOVOnfo4BaG30C-_ug3QunPfQ@mail.gmail.com>
On 2017-11-01 09:52:09, Eric Sunshine wrote:
Show 31 quoted lines
> On Sun, Oct 29, 2017 at 10:51 PM, Antoine Beaupré <anarcat@debian.org> wrote:
>> Virtual namespaces do not correspond to pages in the database and are
>> automatically generated by MediaWiki. It makes little sense,
>> therefore, to fetch pages from those namespaces and the MW API doesn't
>> support listing those pages.
>>
>> According to the documentation, those virtual namespaces are currently
>> "Special" (-1) and "Media" (-2) but we treat all negative namespaces
>> as "virtual" as a future-proofing mechanism.
>
> This patch makes more sense now with the additional commentary.
> Thanks. More below.
>
>> Signed-off-by: Antoine Beaupré <anarcat@debian.org>
>> ---
>> diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl
>> index e7616e1a2..5c85e64b6 100755
>> --- a/contrib/mw-to-git/git-remote-mediawiki.perl
>> +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
>> @@ -264,10 +264,12 @@ sub get_mw_tracked_categories {
>> sub get_mw_tracked_namespaces {
>> my $pages = shift;
>> foreach my $local_namespace (@tracked_namespaces) {
>> + my $namespace_id = get_mw_namespace_id($local_namespace);
>> + next if $namespace_id < 0; # virtual namespaces don't support allpages
>
> Since (it appears) that get_mw_namespace_id() can return undef, you
> probably still need to take that into account before performing a
> numeric comparison:
>
> next if !$namespace_id || $namespace_id < 0;I would argue that this bug exists already elsewhere in the code - no error handling exists there... Furthermore, it should be !defined() because it can be 0.
It might still worth fixing this, but I'm not sure what the process is here - in the latest "what's cooking" Junio said this patchset would be merged in "next". Should I reroll the patchset to fix this or not?
A.
--
N'aimer qu'un seul est barbarie, car c'est au détriment de tous les
autres. Fût-ce l'amour de Dieu.
- Nietzsche, "Par delà le bien et le mal"