launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Hi Brad

Thanks for reviewing.

On 27/04/12 21:59, Brad Crittenden wrote:
> Review: Approve code
> 
> Hi Ian,
> 
> This new email header looks to be a big win.  Thanks for doing the branch.
> 
> I note that you filed the bug that this branch fixes and you don't mention any pre-implementation discussions.  It's comforting as a reviewer to see some hint of collaboration so that it is obvious the developer isn't doing something crazy on his own.  Bear that in mind for the future.
> 

Yeah, because this is part of disclosure, the work has been discussed by
Purple on our standups and normally sinzui picks up the reviews so I was
a little less verbose in the cover letter. Sorry.

> * s/can be made embargoed/can be embargoed
> 
> * It would be nice to make a helper function out of the complicated print statement that appears beginning at lines 111, 158 and 185 of your diff.
> 

I'll fix the above before landing.


> * Thanks for cleaning up the dead code.
> 
> * Have plans been made with the Product Team to announce the availability of this new header via a blog post?  I think our users would be keen to start using it if it is properly publicized.  Talk to Matthew or Dan if you haven't yet.
>

Yes, this is a small part of what we are doing for disclosure. The
transition from individual private/security booleans to information_type
and all that entails has been covered in general in previous blog posts
and when the feature goes beta real soon now there will be more
communication.


-- 
https://code.launchpad.net/~wallyworld/launchpad/bug-mail-information-type/+merge/103793
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.