Review Request 112081: Add reviewboard-am, a tool to apply patches from KDE reviewboard

[Aurélien Gâteau]

> On Aug. 14, 2013, 4:46 p.m., David Edmundson wrote:
> > I'm not sure why I'm singled out as one of the "notable users"; everyone should use this - it's awesome.
> > It is definitely worth including.
> > 
> > I've got some minor comments.

I singled you out because you have been selling reviewboard-am to many KTp users :)


> On Aug. 14, 2013, 4:46 p.m., David Edmundson wrote:
> > reviewboard-am, line 96
> > <http://git.reviewboard.kde.org/r/112081/diff/1/?file=179596#file179596line96>
> >
> >     I don't understand the purpose of this default.
> >     
> >     The vast majority of the times I use reviewboard-am it's because the author doesn't have commit access. They won't then have a @kde email address. 
> >     
> >     It seems to have more risk that I'll just hit enter and accidentally commit something with a really really wrong email address, whereas if you leave the default empty my git push will fail and I'll catch it.

That the fact that the email is not available is a reviewboard privacy setting. One could have a @kde.org address and still not publish it on reviewboard (see the "Keep your user profile private" checkbox in the account page).

Having said so, I am going to make it not provide any default. You are right that it is better to insist on entering something manually. I was actually considering adding code to store a mapping of $username => "$fullname <$email>" between invocations of reviewboard-am but never got around to do so.


> On Aug. 14, 2013, 4:46 p.m., David Edmundson wrote:
> > reviewboard-am, line 127
> > <http://git.reviewboard.kde.org/r/112081/diff/1/?file=179596#file179596line127>
> >
> >     I've had this fail several times; bad review numbers, dodgy internet connection etc.
> >     
> >     A try+catch may make the output a lot less intimidating than the current exception backtraces.

Good point. Will fix.


- Aurélien


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112081/#review37764
-----------------------------------------------------------


On Aug. 14, 2013, 4:10 p.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112081/
> -----------------------------------------------------------
> 
> (Updated Aug. 14, 2013, 4:10 p.m.)
> 
> 
> Review request for kdelibs and David Edmundson.
> 
> 
> Description
> -------
> 
> Add reviewboard-am, a tool to apply patches from KDE reviewboard.
> 
> (subscribing the "kdelibs" group to this request because there seems to be no "kde-dev-scripts" group)
> 
> 
> Diffs
> -----
> 
>   reviewboard-am PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112081/diff/
> 
> 
> Testing
> -------
> 
> I and a few others, most notably David Edmundson, have been using it for quite some time now.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130814/f1186445/attachment.htm>