[Development] New QUrl reviewing

[Stephen Kelly]

On Saturday, March 31, 2012 13:34:52 Thiago Macieira wrote:
> On sábado, 31 de março de 2012 18.14.50, Stephen Kelly wrote:
> > On Friday, March 30, 2012 12:45:44 Thiago Macieira wrote:
> > > On sexta-feira, 30 de março de 2012 17.00.15, Stephen Kelly wrote:
> > > > On Thursday, March 29, 2012 23:31:38 Thiago Macieira wrote:
> > > > > We really need the topic branch reviewing feature back in.
> > > > > It's no
> > > > > fun
> > > > > staging 19, 20, 20 and 21 changes...
> > > > 
> > > > Did you consider staging them a few commits at a time?
> > > 
> > > Yes. The tests might not pass. In fact, the very first commit had a
> > > test failure which was fixed later on and I wasn't about to figure
> > > out what had fixed it.
> > > 
> > > It's all or nothing. That's what the topic branch feature is for.
> > 
> > That means in the future anyone debugging a bug introduced in one of the
> > patches will also get unrelated failures in the unit tests, or maybe
> > won't be able to bisect.
> 
> Yes, then they skip it.

If the bug was introduced in one of your 19, 20, 21 patches, then skipping all 
of them won't do much good.

> 
> > Also definitely not ideal, but you're more likely to be debugging it in
> > the future than me...
> 
> If I can, I try to make sure that each commit is testable on its own.
> 
> However, I will sacrifice testing for atomic changes. I don't like having
> big changes doing lots of things because it's hard to understand how
> certain changes came by.

Agreed. I certainly never suggested you should squash commits together. 

I think making commits which are not testable on their own should be 
discouraged.

Thanks,

-- 
Stephen Kelly <stephen.kelly at kdab.com> | Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
www.kdab.com || Germany +49-30-521325470 || Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-Independent Software Solutions
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20120331/cc056a6a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.qt-project.org/pipermail/development/attachments/20120331/cc056a6a/attachment.sig>