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 BF914138823 for ; Sun, 3 Feb 2013 12:44:39 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 438F021C049; Sun, 3 Feb 2013 12:44:39 +0000 (UTC) Received: from vms173007pub.verizon.net (vms173007pub.verizon.net [206.46.173.7]) by pigeon.gentoo.org (Postfix) with ESMTP id 9E6DF21C049 for ; Sun, 3 Feb 2013 12:44:38 +0000 (UTC) Received: from odin.tremily.us ([unknown] [72.68.84.219]) by vms173007.mailsrvcs.net (Sun Java(tm) System Messaging Server 7u2-7.02 32bit (built Apr 16 2009)) with ESMTPA id <0MHN008EAA2D0U30@vms173007.mailsrvcs.net> for gentoo-catalyst@lists.gentoo.org; Sun, 03 Feb 2013 06:44:38 -0600 (CST) Received: by odin.tremily.us (Postfix, from userid 1000) id CF16D866B67; Sun, 03 Feb 2013 07:44:36 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tremily.us; s=odin; t=1359895476; bh=6EpJepNcLxSVJQIwSmYOLDsqMWTnasUljd7kEuFVCUU=; h=Date:From:To:Subject:References:In-Reply-To; b=AJVLtBdaj8kDKFIleFS5phBS+DZY1rR3/VibBWIUYw8eWXUvugqePu3bI3LeAll1W ll4kC3a4bumD0b5MZ0s8Tsq1J9YM0/SMCaRshybAwRt5hIl/iKFaSQGfjvFipHcnqI fIjr3KEiTmx+Mf+6jg2ptqlglYBTD5r8JnLn1toY= Date: Sun, 03 Feb 2013 07:44:36 -0500 From: "W. Trevor King" To: gentoo-catalyst@lists.gentoo.org Subject: Re: [gentoo-catalyst] More proposed Catalyst changes Message-id: <20130203124436.GB27214@odin.tremily.us> References: <1357633976.4289.61.camel@big_daddy.dol-sen.ca> <1357980958.4289.82.camel@big_daddy.dol-sen.ca> <20130131183922.GA3946@odin.tremily.us> <20130131194653.GA4540@odin.tremily.us> <1359837692.3997.132.camel@big_daddy.dol-sen.ca> 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 Content-type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary=uQr8t48UFsdbeI+V Content-disposition: inline In-reply-to: <1359837692.3997.132.camel@big_daddy.dol-sen.ca> OpenPGP: id=39A2F3FA2AB17E5D8764F388FC29BDCDF15F5BE8; url=http://tremily.us/pubkey.txt User-Agent: Mutt/1.5.21 (2010-09-15) X-Archives-Salt: 7f215240-156a-4728-b329-d0ae815e71a2 X-Archives-Hash: 40e964fcc30b1aa0ee9edaf30dd106e1 --uQr8t48UFsdbeI+V Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Feb 02, 2013 at 12:41:32PM -0800, Brian Dolbec wrote: > On Thu, 2013-01-31 at 14:46 -0500, W. Trevor King wrote: > > 968d818 Initial creation of a defaults file. Split out hash and > > contents to their own classes, files > >=20 > > After this, it's not clear to me what the difference is between > > catalyst.support and catalyst.util. Perhaps they should be merged. >=20 > I hadn't looked at util until now. >=20 > hmm, I wonder if it would be better to move CatalystError there and > rename it to error.py There is only a couple error traceback functions > in it. I was thinking it might be good to define a few more specific > error classes than just the general CatalystError(). But I am not yet > familiar enough with the code to know for certain. This sounds good to me too. > > a4ef493 re-version to "git-rewrite branch" > >=20 > > Why? >=20 > Why not. It is not intended to be pushed into master. It also helps to > confirm that you are running the correct code. I have fixed it so the > code can be run from the git checkout. That way you can have a > "Production" version installed on the same system. Now that I am > testing/debugging, I am comparing catalyst operation and results to > current -9999 code. I am not familiar with using catalyst, so that is > helping me figure out what is wrong. A fair enough. Again, it would be nice if there was a FIXME or something in the commit message so I knew it wasn't destined for master ;). > > 9d752a7 move confdefaults out of main.py > >=20 > > Looks good, except, I'm not sure why you changed from > > `confdefaults.keys()` to `list(confdefaults)` in parse_config() (which > > should probably be living in catalyst.config anyway). >=20 > keeping a separate defaults file can be helpful in importing some info > into different modules while keeping imports to a minimum. Sometimes it > helps prevents circular import problems. At this point I opted for a > separate file, to be determined later if a merge is warranted. ok. > As for list(confdefaults), py3 compatibility. dict.keys() isn't usable > and 2to3 converts it to list(dict)... something about needing to specify > the return type. So is a preemptive change. One less thing to change > later. Really? $ python3.3 -c "a =3D {1:2, 3:4}; print([x for x in a.keys()])" [1, 3] On the other hand, it might be cleaner to just say: for x in confdefaults: But this should still go into a separate commit. > > c303dae some options cleanup, unifying their use, reducing redundancy. > >=20 > > While I like the general thrust of this, I'd be happier with explicit > > boolean options instead of a set of boolean options. For example: > >=20 > > confdefaults =3D { > > 'autoresume': False, > > 'ccache': False, > > =E2=80=A6 > > } >=20 > Yeah, I removed those. They were capitalized versions of the values in > options. So, I optimized them into options becoming a set which > eliminates, the duplication and potential problems by changing the value > of one and not the other. Believe me keeping 2 different lists in sync > can be much more difficult than it seems. It also makes things much > more difficult to debug. (the independent booleans like you suggest can > be considered a list, the other is the options list, set, > string...whatever form it is in) =20 I think I would do something like: import collections as _collections import ConfigParser as _configparser CONFIG =3D _configparser.ConfigParser(dict_type=3D_collections.OrderedDic= t) for setting,value in [ ('ccache', str(False))]: CONFIG.set('DEFAULT', setting, value) although OrderedDict doesn't exist in 2.6, where we should probably just fall back to dicts. Use it with: if CONFIG.getboolean('DEFAULT', 'ccache'): =E2=80=A6 Then there's no duplication to worry about, and you get a configuration object Python developers will be familiar with. > also using member inclusion is faster and prefered compared to other > methods like has_key(). In this case, it is just simpler to use > "options" rather than to individualize them. I don't think option lookup speed will have much impact on catalyst execution speed ;). And with ConfigParser, individual options will require no additional coding. > > Can we use logging instead of print? >=20 > YES!!!!, please :D >=20 > that's been on my wish list too.=20 I can add this if you don't want to. Let me know if you want me to base my patch against `master`, or against something in your branch. > > I think keyword arguments are better, because changes to keywords > > usually occur alongside changes to the argument semantics. A keyword > > mismatch is an obvious fix, while changes due to a semantic shift can > > be more subtle. >=20 > yeah, I was a bit frustrated at that point, debugging code, so chose the > easy way. /me fixes. ;). I've certainly been there too. > > 923e8a2 remove trailing slash for consistency in variables and remove > > extra slashes in paths > >=20 > > os.path.join() >=20 > Yes, for sure. But after I debug my current changes. But also most of > those are for the bash side consistency which can not use os.path.join() > and were adding the slashes again at times. >=20 > I am not a bash programmer, so if someone good at the bash stuff wants > to work on those... go for it. I'll try to keep my damage to a minimum. I can look into this, but I think I need a better description of the problem first. Can you give me an example breakage due to using os.path.join? > Also if releng want to recode them in python, that's ok with me ;) I'd be ok with that too, although I don't feel a pressing need for it ;). > Thank you for the review, it has been informative. And good to keep me > from any blunders. No problem :D Trevor --=20 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 --uQr8t48UFsdbeI+V Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) iQIcBAEBAgAGBQJRDluzAAoJEEUbTsx0l5OMxcIQAK14z5kqkLevc2qkfItACs1c +G/04vZ+WYV0v1vCtvfsN9NWMAkGnmilFObSkfGhpZpdU69iLb1lxX5XtTDDHxrW 1pLUwieFCBH64KHqp605ZlvbxIhZxDHfXjmwFNKGjyiubOAaFNWEXC6MJadrWEQl +3C63faUHGwU6TVOIkhpLq0I2T4yy1aF+MwX8p5S8AvjW9FwSq9KE1C/ntaFBK3v I9cN0Mn4wFbDWLUCrykrdAmOO1ZJ59akBecO4x1ipLuZjcLAO8hjotw8toDXM62x +uA3KJOjmNCV70q1qNjRPr4eiC0atL5ZxwIhmHv9syMdYJkFcUt7375wOgUz0M+K j/SUA0TihKtm7TnC+/8+p4kHKzdJRfw0uFysQzwn6Ly1eE3wzcXezNnou6MCErYP wVlNP+K81W1jl4s+TXbslzuOjnU7pXr8+inFXQiRqHoIkL53Ruj2L7KccakS4a3r WRQsio6AvNCAZ+6YQGn4r6yqq+CJ4YmwdC84r4WJPgs4SnZyzDi5v0waletB7VEv EHjWs2m03KmNwj+pHSQy82ZwSJXoq6N/sb9X4VFg8Tk0psCEnSlbSxEX4xbj8auM YFDcbcxTmxtLigXvghF2qmbhQ+oL/Ih7+GNLP0ATkTBraev9JXeFON5RfEizdSdp qryquXG4P4J2CyIB1F2l =9nNL -----END PGP SIGNATURE----- --uQr8t48UFsdbeI+V--