Re: upload.py -m --message option refactoring (issue 5476044)

[albrec...@googlemail.com]

LGTM - and sorry for the delay, this year's final is very busy :)

We should send an short announcement to the mailing list describing the
changes when this is live.

http://codereview.appspot.com/5476044/diff/5001/upload.py
File upload.py (right):

http://codereview.appspot.com/5476044/diff/5001/upload.py#newcode2250
upload.py:2250: # TODO: [ ] figure out how to send a comment from
upload.py
Maybe the final "task_complete" call I've introduced recently is a good
place to add the message. At least it handles the send_mail flag too.

http://codereview.appspot.com/5476044/

[tech...@gmail.com]

On 2011/12/15 04:59:41, Andi Albrecht wrote:
> LGTM - and sorry for the delay, this year's final is very busy :)

Thanks for finding time to review. =)

> We should send an short announcement to the mailing list describing
the changes
> when this is live.

Will the header of this message suffice for an announcement? I've
already updated wiki page.

> http://codereview.appspot.com/5476044/diff/5001/upload.py
> File upload.py (right):

> http://codereview.appspot.com/5476044/diff/5001/upload.py#newcode2250
> upload.py:2250: # TODO: [ ] figure out how to send a comment from
upload.py
> Maybe the final "task_complete" call I've introduced recently is a
good place to
> add the message. At least it handles the send_mail flag too.

I'm unlikely to get time for this until NY, so I've created an issue
http://code.google.com/p/rietveld/issues/detail?id=351

http://codereview.appspot.com/5476044/

[tech...@gmail.com]

Forgot to mention - committed as
http://code.google.com/p/rietveld/source/detail?r=1aa64f9a92082f11ea69feb348d268ab3265c1a8

http://codereview.appspot.com/5476044/

[Andi Albrecht]

On Fri, Dec 16, 2011 at 10:49 AM, <tech...@gmail.com> wrote:
> On 2011/12/15 04:59:41, Andi Albrecht wrote:
>>
>> LGTM - and sorry for the delay, this year's final is very busy :)
>
>
> Thanks for finding time to review. =)
>
>
>> We should send an short announcement to the mailing list describing
>
> the changes
>>
>> when this is live.
>
>
> Will the header of this message suffice for an announcement? I've
> already updated wiki page.

IMO something similar to the description of the issue with a reference
to the wiki page?

--Andi

[anatoly techtonik]

On Fri, Dec 16, 2011 at 12:53 PM, Andi Albrecht <albrec...@googlemail.com> wrote:

On Fri, Dec 16, 2011 at 10:49 AM,  <tech...@gmail.com> wrote:
> On 2011/12/15 04:59:41, Andi Albrecht wrote:
>>
>> LGTM - and sorry for the delay, this year's final is very busy :)
>
>
> Thanks for finding time to review. =)
>
>
>> We should send an short announcement to the mailing list describing
>
> the changes
>>
>> when this is live.
>
>
> Will the header of this message suffice for an announcement? I've
> already updated wiki page.

IMO something similar to the description of the issue with a reference
to the wiki page?

ANN: upload.py options changed

Many people have found upload.py options for specifying review subject

(-m, --message) and description (-d, --description) confusing, so we've

 changed that to be more intuitive:

1. on first submission
   -t, --title issue subject
   -m, --message issue description
   -F, --file <file> read description from file
2. on issue update
   -t, --title new patchset title
   -m, --message message to reviewers
   -F, --file <file> read message from file

See discussion https://groups.google.com/forum/#!topic/codereview-discuss/bPyZSIfVHDY/discussion for details.

[Andi Albrecht]

It seems like my message when updating an issue got lost somewhere.
I've called upload.py like this to update an issue:

$ ./upload.py -t "Changed keys to arrow keys." -m "Please have another look.
> This version is now live at http://rvtests.appspot.com" -i 5685057 --send_mail --rev qparent

But the message given by the "-m" option is neither on the issue page
nor sent by mail (using upload.py from tip). Or did I miss something?

--
Andi

[anatoly techtonik]

On Tue, Feb 21, 2012 at 8:00 AM, Andi Albrecht
<albrec...@googlemail.com> wrote:
>>
>> 2. on issue update
>>    -t, --title new patchset title
>>    -m, --message message to reviewers
>>    -F, --file <file> read message from file
>
> It seems like my message when updating an issue got lost somewhere.
> I've called upload.py like this to update an issue:
>
> $ ./upload.py -t "Changed keys to arrow keys." -m "Please have another look.
>> This version is now live at http://rvtests.appspot.com" -i 5685057 --send_mail --rev qparent
>
> But the message given by the "-m" option is neither on the issue page
> nor sent by mail (using upload.py from tip). Or did I miss something?

You're right. Options changed, but I run out of time to see how the
hook for sending message can be implemented. The pointer about this is
written in commit message:

http://code.google.com/p/rietveld/source/detail?r=1aa64f9a92082f11ea69feb348d268ab3265c1a8
which leads to
http://code.google.com/p/rietveld/issues/detail?id=351

And TODO is still here
http://code.google.com/p/rietveld/source/browse/upload.py#2262

--
anatoly t.

[Andi Albrecht]

Here's an attempt to fix issue351: http://codereview.appspot.com/5687062

--
Andi

>
> --
> anatoly t.