Open Stack

On Tue, Dec 10, 2013 at 11:45:11AM +0200, Pavlo Shchelokovskyy wrote:
> Hi all,
> I am fixing assertions in unittests now (initially
> https://bugs.launchpad.net/heat/+bug/1259023). I've seen that Clint have
> split this bug into three separate bugs, one per assertion misuse type. As
> these changes will affect many files (practically every file in the tests),
> I would like to ask the following:
> 
> - should I file other separate bugs for other assertion misuse types?
> (right now I also see ignorance of assertIsInstance)

One bug or multiple bugs probably doesn't matter that much IMO, but in
general, I prefer to be more granular, as it makes distributing tasks
accross multiple people, and tracking progress easier.

So +1 for separate bugs, particularly if Clint has already started that
way.

> - should I then commit changes in small patch-sets, may be one-two files at
> once?

In general, split the changes into the smallest logically related chunks,
but don't artificially split it into one or two files per commit, if e.g the
change is really that you sedded all the tests to s/assertFoo/assertBar,
just commit all the changes related to the sedding in one commit.

> - wouldn't it be better to keep all these changes in one bug and fix all
> misuses per file basis (with one file per patch-set for example)? It seems
> to me it would be easier to review in this way.

One file per patch is not a good idea IMO, I think the review burden is
minimised if you make sure that each commit just contains the exact same
change to many files, then it's quick to click through them all and confirm
all looks OK.

Steve