From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.2 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD, UNPARSEABLE_RELAY shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 05DF0202A0 for ; Wed, 1 Nov 2017 16:45:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751660AbdKAQpi convert rfc822-to-8bit (ORCPT ); Wed, 1 Nov 2017 12:45:38 -0400 Received: from marcos.anarc.at ([206.248.172.91]:33676 "EHLO marcos.anarc.at" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751428AbdKAQpi (ORCPT ); Wed, 1 Nov 2017 12:45:38 -0400 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: anarcat) with ESMTPSA id D66D01A00AA From: =?utf-8?Q?Antoine_Beaupr=C3=A9?= To: Eric Sunshine Cc: Git List Subject: Re: [PATCH 4/7] remote-mediawiki: skip virtual namespaces In-Reply-To: Organization: Debian References: <20171029160857.29460-2-anarcat@debian.org> <20171030025142.19421-1-anarcat@debian.org> <20171030025142.19421-5-anarcat@debian.org> Date: Wed, 01 Nov 2017 12:45:36 -0400 Message-ID: <87wp3a54nj.fsf@curie.anarc.at> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On 2017-11-01 09:52:09, Eric Sunshine wrote: > On Sun, Oct 29, 2017 at 10:51 PM, Antoine Beaupré 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é >> --- >> 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"