Code review comment for lp:~bac/launchpad/bug-419930

Revision history for this message
Guilherme Salgado (salgado) wrote :

On Fri, 2010-01-08 at 21:37 +0000, Brad Crittenden wrote:
> > > === modified file 'lib/canonical/launchpad/browser/vocabulary.py'
> > > --- lib/canonical/launchpad/browser/vocabulary.py 2009-07-31 19:38:18
> > +0000
> > > +++ lib/canonical/launchpad/browser/vocabulary.py 2010-01-08 13:36:23
> > +0000
> > > @@ -82,7 +82,7 @@
> > > def person_to_pickerentry(person):
> > > """Adapts IPerson to IPickerEntry."""
> > > extra = default_pickerentry_adapter(person)
> > > - if person.preferredemail is not None:
> > > + if person.preferredemail is not None and not
> > person.hide_email_addresses:
> >
> > I didn't see a test for this; care to add one?
>
> I have discovered Edwin is working to redo the way these pickers are
> working and my drive-by change would soon be replaced by his work.
> Therefore I'm going to back out this portion of the change and just
> wait for his more thorough fix.

Ok, do you mind filing a bug and assigning it to him, just to make sure
it's not forgotten?

>
>
> > > === modified file 'lib/lp/registry/browser/peoplemerge.py'
>
> > > + # We can have multiple email adressess selected, and in
> > this
> >
> > Typo: adressess
>
> This typo was pre-existing. I looked to see if it existed elsewhere and found and fixed six other occurrences.
>

Nice, thanks!

 review approve

--
Guilherme Salgado <email address hidden>

review: Approve

« Back to merge proposal