public inbox for gentoo-catalyst@lists.gentoo.org
 help / color / mirror / Atom feed
From: "W. Trevor King" <wking@tremily.us>
To: gentoo-catalyst@lists.gentoo.org
Subject: Re: [gentoo-catalyst] [PATCH 1/2] modules/generic_stage_target.py, Create SOURCE_MOUNTS_DEFAULTS
Date: Thu, 2 Jan 2014 22:20:52 -0800	[thread overview]
Message-ID: <20140103062052.GJ4680@odin.tremily.us> (raw)
In-Reply-To: <1388728844-2142-2-git-send-email-dolsen@gentoo.org>

[-- Attachment #1: Type: text/plain, Size: 1856 bytes --]

I shift quoted lines around for easier comparison.

On Thu, Jan 02, 2014 at 10:00:43PM -0800, Brian Dolbec wrote:
> +SOURCE_MOUNTS_DEFAULTS = {
> …
> +	"distdir": "/usr/portage/distfiles",
> +	"portdir": "/usr/portage",
> …
> -				"distdir": self.settings["distdir"],
> -				"portdir": normpath("/".join([
> -					self.settings["snapshot_cache_path"],
> -					self.settings["repo_name"],
> -					])),
> …
> +		# initialize our source mounts
> +		self.mountmap = SOURCE_MOUNTS_DEFAULTS.copy()
> +		# update them from settings
> +		self.mountmap["distdir"] = self.settings["distdir"]
> +		self.mountmap["portdir"] = normpath("/".join([
> +			self.settings["snapshot_cache_path"],
> +			self.settings["repo_name"],
> +			]))

Why create dummy initial values and then blow them away?  Wouldn't:

  SOURCE_MOUNTS_DEFAULTS = {
    …
    'distdir': None,  # initialized from settings
    'portdir': None,  # initialized from settings
    }

make more sense?  We'll blow away this default dict once we're loading
it via ConfigParser, so this doesn't have to be super elegant, but
adding values just to clobber them seems misleading.

> -		if "SNAPCACHE" in self.settings:
> -			self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> …
> -		else:
> -			self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
> …
> +		self.mounts = ["proc", "dev", "portdir", "distdir", "port_tmpdir"]
> …
> +		if "SNAPCACHE" not in self.settings:
> +			self.mounts.remove("portdir")

I'd prefer:

  self.mounts = ["proc", "dev", "distdir", "port_tmpdir"]
  if "SNAPCACHE" in self.settings:
      self.mounts.append("portdir")

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

  reply	other threads:[~2014-01-03  6:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-03  6:00 [gentoo-catalyst] Fix the recent python sem_open failure Brian Dolbec
2014-01-03  6:00 ` [gentoo-catalyst] [PATCH 1/2] modules/generic_stage_target.py, Create SOURCE_MOUNTS_DEFAULTS Brian Dolbec
2014-01-03  6:20   ` W. Trevor King [this message]
2014-01-03  6:52     ` Brian Dolbec
2014-01-03 16:11       ` W. Trevor King
2014-01-03  6:00 ` [gentoo-catalyst] [PATCH 2/2] catalyst/targets/generic_stage_target.py: mount /dev/shm on linux Brian Dolbec
2014-01-03  6:33   ` W. Trevor King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140103062052.GJ4680@odin.tremily.us \
    --to=wking@tremily.us \
    --cc=gentoo-catalyst@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox