On Sat, Dec 14, 2013 at 10:05 AM, W. Trevor King <[email protected]> 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
