This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author eric.araujo
Recipients eric.araujo, jafo, meatballhat, tarek
Date 2010-06-01.17:56:48
SpamBayes Score 0.0015393834
Marked as misclassified No
Message-id <1275415012.12.0.149529474732.issue8591@psf.upfronthosting.co.za>
In-reply-to
Content
I’ve started to review your patch but I find it a bid unwieldy to read. Would you mind exporting changesets to patches? I tried to look at your repo, but didn’t know which branch to look at.

So, thanks for giving us the occasion of thinking about the patch workflow. Using changesets seems more natural since we use Bitbucket, while OTOH using regular diffs can streamline the history and remove unnecessary details (e.g. I’m not sure we want branch names to clutter the history forever (see http://stevelosh.com/blog/2009/08/a-guide-to-branching-in-mercurial/ for more info)). Would it be too much of a hassle to provide links or a Mercurial bundle of a set of small changesets? Having different patches would help us separate the non-controversial stuff (whitespace) from the refactoring and improvements.

That said, I can offer a few remarks on the patch.

- Don’t put editor settings in the files.

- Changing “if s in 'yn'” to “if s in ('y', 'n')” is not really an improvement. It’s not more readable to always use tuples or frozensets for membership testing; str has __contains__ for a reason :)

- Not sure how your print_ indirection is helpful.

- I don’t know how much reST we want in the docstrings: None, some constructs, Sphinx extensions?

Don’t worry if there are no replies in a few days, people are pretty busy. I’ll ask about the meta-issues (workflow, reST) on the ML if Tarek doesn’t answer here. Cheers :)
History
Date User Action Args
2010-06-01 17:56:52eric.araujosetrecipients: + eric.araujo, jafo, tarek, meatballhat
2010-06-01 17:56:52eric.araujosetmessageid: <1275415012.12.0.149529474732.issue8591@psf.upfronthosting.co.za>
2010-06-01 17:56:49eric.araujolinkissue8591 messages
2010-06-01 17:56:48eric.araujocreate