txaws-dev team mailing list archive

[Duncan McGreggor <duncan@xxxxxxxxxxxxx>]

Review: Needs Fixing

Wow! You resuscitated that old branch... amazing!

So, I haven't had a chance to look at this yet, and I should... given than I started it ;-) However, after quickly scrolling though the diff, it seems that you've done something entirely sensible: put the test payloads in .xml files.

However, what we've done in the rest of txaws is put these in txaws.testing.payloads. By having them in Python strings, we're able to do string substitution for things like version numbers, etc.

It be best to use the same approach that the rest of txaws uses.

(However, taking a quick peek inside payloads right now, it looks like *that* could use a major overhaul, too... not all payloads are named such that it's immediately obvious to which part of AWS they pertain. I wouldn't mind seeing that module split into a subpackage, e.g., payloads/common.py, payloads/ec2.py, payloads/s3.py, etc. But that's work for another branch!)

Anyway, let's start with Robert's feedback and the payload feedback, and then we can re-tackle a review of the updated merge request.
-- 
https://code.launchpad.net/~txaws-dev/txaws/415691-add-simpledb/+merge/88530
Your team txAWS Developers is subscribed to branch lp:~txaws-dev/txaws/415691-add-simpledb.