Code review comment for lp:~mabac/launchpad-work-items-tracker/better-config

Revision history for this message
Mattias Backman (mabac) wrote :

On Fri, Feb 3, 2012 at 2:06 PM, Guilherme Salgado
<email address hidden> wrote:
> Review: Approve
>
> This all looks good to me; I've just two comments.
>
> First, I think the multimedia contact should be tom gall and not michael hope, no?

You're right.

>
> Second, we need to add contact addresses for the linaro project as well:
>  # Linaro project to track
>  project = 'linaro'
>  project_series = 'trunk'
>
> But given that's not in extra_projects and we don't want it to be there, we should probably just add an entry for it at the end of project_notification_addresses instead of using a class for it.

Yes, I forgot that project. Will add it like that.

>
> Oh, and the indentation on line 207 seems to be off by one space.

I shouldn't tell you that it actually was a tab...

>
> Oh, and it'd be good to manually test that extra_projects in this config is identical to extra_projects in the old one; it should be trivial to load_config() both and compare extra_projects in both, right?

Absolutely. I'll try that before giving it a spin on staging.

> --
> https://code.launchpad.net/~mabac/launchpad-work-items-tracker/better-config/+merge/91411
> You are the owner of lp:~mabac/launchpad-work-items-tracker/better-config.

« Back to merge proposal