#1131 closed enhancement (fixed)

handful of tidy-up patches from 1.7.1 cycle

Reported by: zooko Owned by:
Priority: minor Milestone: 1.8β
Component: code Version: 1.7.0
Keywords: cleanup easy reviewed Cc:
Launchpad Bug:

Description

I have three patches here which I chose to keep out of trunk during 1.7.1.

Please review!

Attachments (2)

three_cleanups.dpatch.txt (15.8 KB) - added by zooko at 2010-07-20T04:27:50Z.
tidy-up-logging.dpatch.txt (24.0 KB) - added by zooko at 2010-08-02T06:46:48Z.

Download all attachments as: .zip

Change History (11)

Changed at 2010-07-20T04:27:50Z by zooko

comment:1 Changed at 2010-07-20T05:11:10Z by davidsarah

  • Owner changed from somebody to davidsarah
  • Status changed from new to assigned

comment:2 follow-up: Changed at 2010-07-20T05:25:51Z by davidsarah

Please wrap the lines longer than about 100 chars.

Also, random_unicode seems more general than it needs to be. (It would probably be slightly simpler to choose random unicode code points, rather than to choose a random UTF-8 encoding for each character and loop until it is valid, since the encoded length doesn't appear to be critical.)

Otherwise looks fine. Please keep this ticket open so I can attach my own cleanup patches :-)

comment:3 in reply to: ↑ 2 Changed at 2010-07-21T16:51:23Z by zooko

Replying to davidsarah:

Please wrap the lines longer than about 100 chars.

Okay (grumble).

Also, random_unicode seems more general than it needs to be. (It would probably be slightly simpler to choose random unicode code points,

Yeah, for the purposes of this benchmark, it could even just be a random ascii string or something. How would one choose random unicode code points other than the way random_unicode() does it?

comment:4 Changed at 2010-07-23T05:34:10Z by zooko

  • Keywords review-needed removed

Removing review-needed. I'll line-wrap, probably leave the silly random_unicode() as is, and commit the patch. Leaving this ticket open for other post-1.7.1 cleanup patches. :-)

comment:5 Changed at 2010-08-02T04:37:54Z by davidsarah

  • Keywords reviewed added

There's an uncommented print statement in is_happy_enough. Otherwise fine.

comment:6 Changed at 2010-08-02T04:38:12Z by davidsarah

  • Owner davidsarah deleted
  • Status changed from assigned to new

comment:7 Changed at 2010-08-02T06:46:16Z by zooko

Below is a patch to reformat lines to < 100 chars and to tidy up logging in immutable/upload.py.

Changed at 2010-08-02T06:46:48Z by zooko

comment:8 Changed at 2010-08-03T18:33:01Z by davidsarah

  • Resolution set to fixed
  • Status changed from new to closed

Applied for 1.8beta in [4632/ticket798], [4634/ticket798], [4637/ticket798]. Uncommented print statement removed in [4636/ticket798].

comment:9 Changed at 2010-08-03T18:34:14Z by davidsarah

... and [4633/ticket798].

Note: See TracTickets for help on using tickets.