From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58080 invoked by alias); 30 Jun 2015 17:14:36 -0000 Mailing-List: contact cygwin-apps-help@cygwin.com; run by ezmlm Precedence: bulk Sender: cygwin-apps-owner@cygwin.com List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Mail-Followup-To: cygwin-apps@cygwin.com Received: (qmail 58069 invoked by uid 89); 30 Jun 2015 17:14:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-in-17.arcor-online.net Received: from mail-in-17.arcor-online.net (HELO mail-in-17.arcor-online.net) (151.189.21.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Tue, 30 Jun 2015 17:14:33 +0000 Received: from mail-in-10-z2.arcor-online.net (mail-in-10-z2.arcor-online.net [151.189.8.27]) by mx.arcor.de (Postfix) with ESMTP id 3mLXNk0hX6z6GwT for ; Tue, 30 Jun 2015 19:14:30 +0200 (CEST) Received: from mail-in-04.arcor-online.net (mail-in-04.arcor-online.net [151.189.21.44]) by mail-in-10-z2.arcor-online.net (Postfix) with ESMTP id 1462B28B77A for ; Tue, 30 Jun 2015 19:14:30 +0200 (CEST) X-DKIM: Sendmail DKIM Filter v2.8.2 mail-in-04.arcor-online.net 3mLXNj6bjQz5F06 Received: from Gertrud (p54B46861.dip0.t-ipconnect.de [84.180.104.97]) (Authenticated sender: stromeko@arcor.de) by mail-in-04.arcor-online.net (Postfix) with ESMTPSA id 3mLXNj6bjQz5F06 for ; Tue, 30 Jun 2015 19:14:29 +0200 (CEST) From: Achim Gratz To: cygwin-apps@cygwin.com Subject: Re: setup References: <20150610080526.GC31537@calimero.vinschen.de> <871thjtq0m.fsf@Rainer.invalid> <20150610185417.GL31537@calimero.vinschen.de> <87wpzbs2yj.fsf@Rainer.invalid> <87mw060xg3.fsf@Rainer.invalid> <20150612102945.GS31537@calimero.vinschen.de> <87bngkzwki.fsf@Rainer.invalid> <87ioa8t0ha.fsf@Rainer.invalid> <20150629134154.GA2918@calimero.vinschen.de> <873819okox.fsf@Rainer.invalid> <20150630164052.GE2918@calimero.vinschen.de> Date: Tue, 30 Jun 2015 17:14:00 -0000 In-Reply-To: <20150630164052.GE2918@calimero.vinschen.de> (Corinna Vinschen's message of "Tue, 30 Jun 2015 18:40:52 +0200") Message-ID: <87bnfxm83u.fsf@Rainer.invalid> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2015-06/txt/msg00159.txt.bz2 Corinna Vinschen writes: > Ok, for once. But please make sure that you split the commit into > functional chunks next time it's so large. Well, the original patch was a lot smaller and I didn't really expect that I'd have to replace such a large chunk of the old code to make things work correctly. I've thought about it some more and I guess I can split off the search implementation in fromcwd. The meat of the patch would still be quite large, though. > And send it to this list, so > code snippets can be referenced in the review. Sure. > A few more nits, mostly on style: > > - What I'm missing are code comments in do_local_ini and do_remote_ini > describing what the new methods do. Yes, the old code didn't have a > lot of comments either, but it would be nice to have this better > explained for future reference. I'll add some. > - Do you plan to keep the DEBUG_FROMCWD bracketed code in there? If so, > it should probably just use `#if DEBUG' as elsewhere. I was using similar code from crypto.cc as a guide. I can remove that code as it's debugged now. In the long run it'd be better to have Log(DEBUG_...) calls that get optimized out when doing the final build indtead of conditional compilation. > - The call to yydebug=1 is almost invisible now. Please revert that > to the old > > [empty line] > /*yydebug = 1; */ > [empty line] OK. > - ini_decompress as a function name may be a bit misleading. What > about decompress_ini_file instead? OK. > Otherwise it looks ok, especially splitting the code into the new > functions ini_decompress/decompress_ini_file and check_ini_sig and > removing IniParseFindVisitor is a blessing. I'd have really liked to refactor local and remote as well as they are almost exact copies of each other. I didn't try if file:// URLs would have worked, maybe they do. If so, the search part for the remote init would need to be lifted so that both code paths would just process a list of URLs. Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ Wavetables for the Terratec KOMPLEXER: http://Synth.Stromeko.net/Downloads.html#KomplexerWaves