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 0101C138823 for ; Sun, 3 Feb 2013 12:21:13 +0000 (UTC) Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id 8603AE064C; Sun, 3 Feb 2013 12:21:12 +0000 (UTC) Received: from vms173013pub.verizon.net (vms173013pub.verizon.net [206.46.173.13]) by pigeon.gentoo.org (Postfix) with ESMTP id EF6F4E064C for ; Sun, 3 Feb 2013 12:21:11 +0000 (UTC) Received: from odin.tremily.us ([unknown] [72.68.84.219]) by vms173013.mailsrvcs.net (Sun Java(tm) System Messaging Server 7u2-7.02 32bit (built Apr 16 2009)) with ESMTPA id <0MHN002SZ8YWVI70@vms173013.mailsrvcs.net> for gentoo-catalyst@lists.gentoo.org; Sun, 03 Feb 2013 06:20:57 -0600 (CST) Received: by odin.tremily.us (Postfix, from userid 1000) id 350A5866B12; Sun, 03 Feb 2013 07:20:56 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tremily.us; s=odin; t=1359894056; bh=9g9rNsuxAe2a9LfpSq3Ye0lOgiva0YeAB4YptPv6I1A=; h=Date:From:To:Subject:References:In-Reply-To; b=UPveXAeX/XVOGcjzitLg9IO4fe8NxCV0gq+hoASAiUjs+gu2BSjvqRJjeXuRGne6X aDFMN4tCFx4EAG1orRA6iVOREqHKecRxcVgCdWa/JzEuw3TLFj6uQHu3ZlVYziHziq LZPUAWsuZj/PwTzPyUZAWG08gxO3jLRO5qssbZRo= Date: Sun, 03 Feb 2013 07:20:56 -0500 From: "W. Trevor King" To: gentoo-catalyst@lists.gentoo.org Subject: Re: [gentoo-catalyst] More proposed Catalyst changes Message-id: <20130203122056.GA27214@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> <1359830719.3997.63.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=ZPt4rx8FFjLCG7dd Content-disposition: inline In-reply-to: <1359830719.3997.63.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: 2332ffa2-f761-4ff0-8d82-a91c120c7d12 X-Archives-Hash: 843388e537456271ae34811a12856eaa --ZPt4rx8FFjLCG7dd Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Feb 02, 2013 at 10:45:19AM -0800, Brian Dolbec wrote: > I do think that it will be difficult to make complete commits for > some changes that do not break catalyst until the remaining commits > are also merged. This may be true for some of the shifting commits, we'll see after the initial reroll. > Thank you for enough info for me to google about commit summary, > message body. Until now I didn't know about needing to separate > them with a blank line. I had never come across that info before, > so had given up trying to format them better. Everything I tried > had failed. Even googling it now it seems to be everywhere but > git's own help. Actually, it is in Git's help: $ git commit --help | grep -B3 blank DISCUSSION Though not required, it=E2=80=99s a good idea to begin the commit messa= ge with a single short (less than 50 character) line summarizing the change, followed by a blank line and then a more thorough description. The text up to the first blank line in a commit message is treated as the commit > > 321df3b whitespace cleanup > >=20 > > This looks good, but there are also whitespace cleanups in some other > > commits (e.g. 2d36d29). I think we should just run something like: > >=20 > > for FILE in $(git ls-tree -r --name-only HEAD | grep -v 'bz2$'); do > > sed -i 's/[[:space:]]*$//' "$FILE" > > done > >=20 > > and have done with it ;). >=20 > yeah, that requires way more knowledge about those tools than I know. > For me, I open a file, the editor does the cleanup automatically. If I > don't separate them when committing, then... If I didn't edit the > file... I've pushed this change to the `whitespace` branch on git://tremily.us/catalyst.git if you want to rebase your changes onto that. I can also submit it as a patch to the mailing list if anyone wants it that way. > > 38cd3bb add more configured defaults > >=20 > > List the new settings (distdir, repo_name, packagedir, port_tmpdir, > > options, snapshot_name) in the commit message so I don't have to read > > the diff. Probably explain why you think they should be configurable > > as well. `options` is also a pretty ambiguous name. >=20 > Now that I know how to blab on in a message body with a brief summary, > yeah :) >=20 > "options" was existing and seems comparable to FEATURES for emerge in > make.conf. Suggestions for a new name? In a later comment I suggested breaking each of the options variables out into their own setting. Then we don't have to worry about what to call the boolean settings in aggregate ;). > > This might also be a good place to move the path construction over to > > use os.path.join(). >=20 > Yes, I agree, but I am still getting accustomed to the code, so was > holding off that for a bit. Need to debug changes first. I am also > contemplating stealing the path() from layman which joins a list of > partial paths. I've noticed it could simplify code in many places that > join more than 2 partial paths. Doesn't os.path.join already do that? $ python2.7 -c "import os; print(os.path.join('a', 'b', 'c'))" a/b/c > > This might also be a good time to transition from `CatalystError, =E2= =80=A6` > > to `CatalystError(=E2=80=A6)` where you're touching lines. > >=20 > > I'm not sure which versions of Python Catalyst is trying to support, > > but I'd be happy to see the beginings of a migration to '{}'.format() > > for building strings (vs. the current `'' + ''` concatenation) >=20 > yeah, there are a ton of those in the code. >=20 > Since portage is python-2.6+ and python-2.5 is pretty close to being > removed from the tree... I intend to make it, 2.6, 2.7, 3.2 + > compatible.=20 So the CatalystError(=E2=80=A6) updates will be fine, and we should use '{0}'.format(xyz). > > 60919e4 Apply all Matt Turner's has_key() replacement patches > >=20 > > This looks like a job for `rebase` ;). Cherry-picking will just lead > > to redundant commits. >=20 > That will be looked after in my rebase to master again. Since I had > already made a number of changes in my rewrite branch, I could not apply > his commit cleanly without editing. Right. What you should have done is rebase onto the new `master` branch. Where Matt's changes clashed with yours, Git would stop and ask to to fix the conflicts. Then you could update *your* patches so that they worked ;). Since Matt's changes got into `master` first, he gets to keep them the way they originally were. > > 7a909b9 cleanup long lines, improve useage() output formatting > slightly > >=20 > > Can we just switch from getopt to argparse (Python =E2=89=A52.7)? >=20 > Yes, we can also add argparse as dep for 2.6 if they want to maintain > 2.6 compatibility. Ah, that would be much nicer :). > > 5eeb8b1 new minimal start script > >=20 > > I think __version__ and __maintainer__ should live in > > catalyst/__init__.py. >=20 > Can do. The version can also be set for releases easily using some > methods from gentoolkit when making the release tarball. The version > could then be left as "git" with a partial hash added of the last commit > when it was merged with. I do something like this in bugs-everywhere [1], but for Catalyst I think it may be more work than it's worth. > > 267dbb6 add __init__.py's to modules and arch sub-pkgs > >=20 > > Added with blank lines? >=20 > I cheated at the time and just used my editor to save an existing one to > different places. So it likely saved it with a blank line. Instead of > using a terminal and touch. These should probably get copyright blurbs and descriptive docstrings anyway, so it's probably not worth worrying over the blank lines. > > da6cbbf rename shell script targets directory to targets.sh so as to > not be confused with the new python targets directory > >=20 > > =E2=80=9Cso as=E2=80=A6=E2=80=9D should go in the commit message body t= o get a shorter > > summary. > >=20 > > I'm not sold on this. Since they are data files intended to be used > > by the Python modules, why not move them to catalyst/targets/scripts > [1]? > =20 > I had been re-thinking that change and was thinking of renaming it back > to targets now that things have been better rearranged. There is one py > file in stage1. It is also easier to do now since it is just one edit > and a git mv away. I've removed that directory name from being hard > coded in the scripts, so now it just means there is one place to edit. This sounds reasonable to me. > > f9f18be update gitignore > >=20 > > The Scratch, catalystc, *.geany, and test* entries would appear to be > > specific to your local installation and usage. I'd drop them from the > > reroll. > >=20 >=20 > They are only for my dev space git repo and me. They will not be part > of a final ready to merge branch. But my dev space is also acting as my > backup, so I committed them. It makes my git status output cleaner. Fair enough, but it's probably a good idea to put your local changes in a clearly marked commit: $ git commit -am 'FIXME: local changes to gitignore' > > d23d96b revert urllib import from commit > 64c16cae70da13de3c55d8555a2e4c5dcdf2fcad since it is not used anywhere > in catalyst > >=20 > > 64c16cae does a lot more than import urllib, so I think =E2=80=9Crevert= =E2=80=9D gives > > the wrong impression. I'd remove the import entirely (instead of > > commenting it out), and say: > >=20 > > catalyst/support.py: Remove unused urllib import > >=20 > > This was introduced in commit > > 64c16cae70da13de3c55d8555a2e4c5dcdf2fcad but is no longer used. >=20 > Actually urllib was never used. It was added as a fix for a bug. It > was not a correct fix, so just masked what was causing the bug. I only > commented it out until I could either find what caused the bug or it > just didn't return due to other changes. It is often easier to > comment/uncomment during testing runs than to remove/re-add code. Sure. It's strange that it had any effect at all on a bug if it was never used, but whatever ;). Cheers, Trevor [1]: http://gitorious.org/be/be/blobs/master/libbe/version.py#line21 http://gitorious.org/be/be/blobs/master/Makefile#line80 --=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 --ZPt4rx8FFjLCG7dd Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.19 (GNU/Linux) iQIcBAEBAgAGBQJRDlYlAAoJEEUbTsx0l5OMMzkP/26EFelOSbjZ4ScmdmGVHM6L KQxDNTskY/CzcM/j0mwO7L3WAwmgq4qyEKVMqHZEbyGMGUU5xTe8ertEsCTPt2HP H7NfK+FNlYm1Ybcq+Z9zlp9BH5efru3ZPfrDWhCK6r4Ybd7gPnb5vmke6602a6ra AGHDzf0sT83XkV8/69cQfmSlaFbsVNcCZxGpJYXy13sm6m0Z4N2zIcWW10MLjlgX bZauT/dLyeYrAxnCFebVqQsIWIjTAUWneKSDm9IplsusliFgv6dGpYSTqt3VHKTm 5j4e0UJvmd25vLdp8/sUbt77xja7Vv/UegS4zSdGUV5V41s7mkYHkCOnXnTh+0gm ic9TO9U0mG83TcJUMDTeFi45wwFAqejjmIPLUk3o81F56maYQDiZUbP6wYrz4fGI EFSSaN9AqxlC8v7B0yrE5A6ondgAfHggL4YKKZdLnsTCJdQFklG36ejPwIZviVMO WBNXFtNYdBOhnP2broOvp2mNuVSGJs1Ba2U3ETIq2B+HVlL0Aeghu3O3JP7Hi/+Q iZTx9gWjwizk6qyStnc0vUA2ozkMwU18Qre966JO3/1kV47exRsKE9o68Uk/3oq6 JyKZDyGKoFsGsRZ1y9s5Gpf+qyQ1GQ+AKzcT+BbCC2re22vqntOBALmgn+wbdLki K3SEhIq9icm/Fcb2E9j2 =+rIu -----END PGP SIGNATURE----- --ZPt4rx8FFjLCG7dd--