728778 - Add feedback flag to attachments

Status: RESOLVED FIXED

(Developer Services :: Mercurial: bzexport, defect)

Description

[Josh Matthews [:jdm]]

Sometimes you don't want a full review, but then you have to use the bugzilla interface and you become sad.

Comment 1

[Steve Fink [:sfink] [:s:]]

Attachment: Implement CC and feedback, also switch reviewers to comma-separated instead of fixed list of 2

This applies on top of a pep8-compatibility patch. I should land that.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Implement CC and feedback, also switch reviewers to comma-separated instead of fixed list of 2

Sorry, I've been sitting on this one for a while, mostly due to the lack of a test harness. But I've been using it long enough now, and Gijs just started implementing this separately, so I think it's time to land. Note that I did a fairly brutal rebase to get this one in landing position; I don't *think* I messed it up.

Maybe I should just land |hg pushed| and not tell anyone about it. It's truly awful, though. And yet I use it all the time... :( Rebasing past it is no fun.

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Implement CC and feedback, also switch reviewers to comma-separated instead of fixed list of 2

Doh! Lost the bz.py changes entirely in the rebase. (Fortunately still had them on the initial patch posted to this bug.)

Patch juggling is not going well for me today. Especially since I just discovered that my bzexport patch queue is not versioned. Need to add an mqext option to warn about that.

Comment 4

[Ed Morley [:emorley]]

Comment on attachment 809450 [details] [diff] [review]
Implement CC and feedback, also switch reviewers to comma-separated instead of fixed list of 2

I can't seem to get this to apply against the repo tip - do I need to apply a patch from one of the other bugs first?

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: Implement CC and feedback, also switch reviewers to comma-separated instead of fixed list of 2

Hm, I must've rebased the patch at some point in the past, because I currently have it applied to the tip. Re-uploading.

Comment 6

[Ed Morley [:emorley]]

Comment on attachment 821126 [details] [diff] [review]
Implement CC and feedback, also switch reviewers to comma-separated instead of fixed list of 2

Review of attachment 821126 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the 'flag_names[0]' corrected :-)

::: __init__.py
@@ +698,5 @@
>  
>      flag_ids = [ id for id in prodflags if flagdefs[str(id)]['name'] == flag_name ]
>  
>      if len(flag_ids) != 1:
> +        raise util.Abort(_("Could not find unique %s flag id") % flag_names[0])

Do you mean 'flag_name'?

Comment 7

[Steve Fink [:sfink] [:s:]]

(In reply to Ed Morley [:edmorley UTC+1] from comment #6)
> Comment on attachment 821126 [details] [diff] [review]
> Implement CC and feedback, also switch reviewers to comma-separated instead
> of fixed list of 2
> 
> Review of attachment 821126 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with the 'flag_names[0]' corrected :-)
> 
> ::: __init__.py
> @@ +698,5 @@
> >  
> >      flag_ids = [ id for id in prodflags if flagdefs[str(id)]['name'] == flag_name ]
> >  
> >      if len(flag_ids) != 1:
> > +        raise util.Abort(_("Could not find unique %s flag id") % flag_names[0])
> 
> Do you mean 'flag_name'?

The patch explicitly switches *away* from flag_name. Which makes no sense at all.

Can I blame it on rebasing? Or maybe plead insanity.

Anyway, thanks! Fixed.

Comment 8

[Steve Fink [:sfink] [:s:]]

http://hg.mozilla.org/users/tmielczarek_mozilla.com/bzexport/rev/0cdc150b1562