I agree that stylistically it's better, but it's equivalent and munging the
test beyond search/replace seems out of scope (and would begin a tear which
would take quite some time to finish). I guess I could write a sed type
script to do it, but I'd worry about indentation screw ups.


On Wed, Jul 31, 2013 at 11:04 AM, David Baumgold
<notifications@github.com>wrote:

> In common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py:
>
> >          self.assertRaises(ItemNotFoundError,
> >                            modulestore().get_course,
> > -                          CourseLocator(course_id='GreekHero', revision='published'))
> > +                          CourseLocator(course_id='GreekHero', branch='published'))
>
> Nitpick: as long as you're modifying this code, I much prefer the context
> manager form of the assertRaises statement:
>
> with self.assertRaises(ItemNotFoundError):
>     modulestore().get_course(CourseLocator(course_id='GreekHero', branch='published'))
>
> Much more readable, and you can execute multiple statements in the block.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/edx/edx-platform/pull/528/files#r5505665>
> .
>