From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lists.gentoo.org (pigeon.gentoo.org [208.92.234.80]) by finch.gentoo.org (Postfix) with ESMTP id 7454A138247 for ; Sat, 14 Dec 2013 18:44:38 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 067DBE0B04; Sat, 14 Dec 2013 18:44:38 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by pigeon.gentoo.org (Postfix) with ESMTPS id 74DC8E0B04 for ; Sat, 14 Dec 2013 18:44:36 +0000 (UTC) Received: from mail-qc0-f177.google.com (mail-qc0-f177.google.com [209.85.216.177]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: mattst88) by smtp.gentoo.org (Postfix) with ESMTPSA id A282633F551 for ; Sat, 14 Dec 2013 18:44:35 +0000 (UTC) Received: by mail-qc0-f177.google.com with SMTP id m20so2528740qcx.36 for ; Sat, 14 Dec 2013 10:44:33 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :content-type; bh=aFQFm+qSosaknLJ5vhkMPdSJzzwdfzXKovocsKvOmNc=; b=OwjISxJsDHDmo6H4IWmHL02lD3DinpvR9rsay3NrmcoZtRvSSrJ4CiUAIYcFhSWHPS JqguYvhlQp9qNb4I5OipZ1uwkH8yLgPRbhcslxo59tukKYVwj9Io2FwgKLSmdYJ1xqEs ubrMDDU6O8C8d5TObRBcs9J0FFSuOpixK7n2wJXfVWZV7MJs5SfnWQGPMxVAqfh9oo1g znviNpOfbVpLXu6hmxs4JQUb3zHoFGyXyaGttPybsQ62PbLABOI+mSBD3TImqUhi2ffK gQ3gebGIvN8Om0xqtg4rr3AX5QGsQ6H0NKzkPCbqfUxhoC8HrkXGsQM2uZ102hGHf6iy W3YA== X-Received: by 10.224.80.4 with SMTP id r4mr16809075qak.69.1387046673912; Sat, 14 Dec 2013 10:44:33 -0800 (PST) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-catalyst@lists.gentoo.org Reply-to: gentoo-catalyst@lists.gentoo.org MIME-Version: 1.0 Received: by 10.229.178.129 with HTTP; Sat, 14 Dec 2013 10:44:13 -0800 (PST) In-Reply-To: <20131214180542.GA25409@odin.tremily.us> References: <1386990436-9198-1-git-send-email-dolsen@gentoo.org> <1386990436-9198-3-git-send-email-dolsen@gentoo.org> <1387019742.3897.121.camel@big_daddy.dol-sen.ca> <20131214163714.GY25409@odin.tremily.us> <20131214180542.GA25409@odin.tremily.us> From: Matt Turner Date: Sat, 14 Dec 2013 10:44:13 -0800 Message-ID: Subject: Re: [gentoo-catalyst] [PATCH 2/4] Remove self.mounts and self.mountmap's use of paths for keys and paths. To: gentoo-catalyst@lists.gentoo.org Content-Type: text/plain; charset=ISO-8859-1 X-Archives-Salt: db8eaef3-9dad-4ea3-b8c4-572714d202dd X-Archives-Hash: c1c7b93bbd9ecb636a864fa2de02af3a On Sat, Dec 14, 2013 at 10:05 AM, W. Trevor King wrote: >> On Sat, Dec 14, 2013 at 03:15:42AM -0800, Brian Dolbec wrote: > And here's the request-pull: > > $ git request-pull origin/master wking wking/dolsen-rewrite-part-1 > The following changes since commit 1c86c64113491885b159529dacb452ce6a3e5f4b: > > catalyst 2.0.15 (2013-11-13 13:59:25 -0800) > > are available in the git repository at: > > git://tremily.us/catalyst.git dolsen-rewrite-part-1 > > for you to fetch changes up to 907c35fb1e1cae75cc78d45b054c3c43ac9deb48: > > cleanup long lines, improve usage() output formatting slightly (2013-12-14 09:48:22 -0800) > > ---------------------------------------------------------------- > Brian Dolbec (20): > modules/tinderbox_target.py: Use 'portdir' instead of hard-coding '/usr/portage' > modules/generic_stage_target.py: Use 'portdir' instead of hard-coding '/usr/portage' > modules/generic_stage_target.py: Use 'portdir' instead of hard-coding '/usr/portage' > modules/generic_stage_target.py: Use 'distdir' instead of hard-coding '${PORTAGE}/distfiles' > modules/generic_stage_target.py: Use a 'local_overlay' setting instead of hard-coding '/usr/local/portage' > catalyst: Split confdefaults into line-per-entry > catalyst: Add 'repo_name' default > catalyst: Add 'snapshot_name' default > catalyst: Add 'packagedir' default instead of hard-coding '/usr/portage/packages' > catalyst: Add 'port_tmpdir' default instead of hard-coding '/var/tmp/portage' > modules/generic_stage_target.py: Don't use paths as mountmap keys > modules/generic_stage_target.py: Use 'proc' instead of '/proc' as the mountmap key > modules/generic_stage_target.py: Use 'dev' instead of '/dev' as the mountmap key > modules/generic_stage_target.py: Use 'distdir' instead of '/usr/portage/distfiles' as the mountmap key > modules/generic_stage_target.py: Use 'port_tmpdir' instead of '/var/tmp/portage' as the mountmap key > modules/generic_stage_target.py: Use 'devpts' instead of '/dev/pts' as the mountmap key > modules/generic_stage_target.py: Use 'packagedir' instead of '/usr/portage/packages' as the mountmap key > modules/generic_stage_target.py: Use 'kerncache' instead of '/tmp/kerncache' as the mountmap key > modules/generic_stage_target.py: Use 'ccache' instead of '/var/tmp/ccache' as the mountmap key > cleanup long lines, improve usage() output formatting slightly > > catalyst | 101 ++++++++++++++++++++++++---------------- > modules/generic_stage_target.py | 92 ++++++++++++++++++------------------ > modules/snapshot_target.py | 14 ++++-- > modules/tinderbox_target.py | 4 +- > 4 files changed, 119 insertions(+), 92 deletions(-) > > I can submit them to the list via `git send-email` if you'd like, just > let me know. > > Cheers, > Trevor Awesome, thanks for doing this. This is exactly how this patch series should look and was super easy to review. I feel they're basically ready to go, but I want everyone to be in the habit of sending patches to the list, so please do. Preemptive review: I saw just a few minor things, almost entirely dealing with lack of whitespace around operators. git log -p --reverse origin/master..wking/dolsen-rewrite-part-1 and then search for ^\+.*[^ ]\+[^ ] (less regex) finds a couple. I'd opt to just put parenthesis around prints where we're going to be line wrapping its argument. It accomplishes the same thing as the trailing slash, but moves us slightly in the direction of python3. So, fix up lines you think it possible that match ^\+.*\\$ The other thing: I think we should drop most of the last patch for now. Wrapping long lines before we switch from 8-space tabs to PEP8 4-space style seems premature. The other half of that commit that replaces a series of prints with a single one looks good though. You may want to put your Signed-off-by on the patches to note that you refactored them heavily. Thanks again, Matt