release plan and request for merge

[C. Boemann]

On Thursday 23 February 2012 11:57:56 Elvis Stansvik wrote:
> 2012/2/23 C. Boemann <cbo at boemann.dk>:
> > Hi all
> > 
> > So the minisprint on undo in Words is over and we had success. I'm
> > requesting merge of our branch but more on that below.
> > 
> > Assuming we merge within a day or two, I propose we create a release
> > branch one of the next days. And on next friday we tag an RC from that
> > release branch. Then 3 weeks and tag the final.
> > 
> > As for the merging of the text-undo-minisprint branch Pierre and I worked
> > mostly in tandem so we have had constant peer review. However sitting so
> > close we could be the victim of group-think so I would be glad if
> > someone else could look through the branch. Creating a review request,
> > can be done but since we have like moved everything it doesn't make much
> > sense.
> 
> Glad to hear the sprint was a success.
> 
> I'm definitely not savvy enough with the code and don't know enough
> about the problems you've been solving to do a thorough review, so I
> just have a few nitpicks:
> 
>  const QUrl KoTextDocument::FrameCharFormatUrl =
> QUrl("kotext://frameCharFormat");
> +const QUrl KoTextDocument::ShapeControllerURL =
> QUrl("kotext://shapeController");
> 
> s/URL/Url/
> 
> +        IndexGeneratorManager ,
> +        FrameCharFormat ,
> +        ShapeController
> 
> s/ ,/,/
> 
> -void KoTextEditor::deleteChar(MoveOperation direction,
> KoShapeController *shapeController)
> +void KoTextEditor::deleteChar(bool previous, KUndo2Command *parent)
> 
> Why change from enum -> bool for the direction here? IMHO enum tells a
> richer story when you're looking at a random call deleteChar
> somewhere, even though it'll just be two values.
> 
> +        d->updateState(KoTextEditor::Private::Custom,
> i18nc("(qtundo-format)", "Insert Footnote"));
> +    }
> +    else {
> 
> "else" on same line as }. (Same in a few other places to which it has
> probably propagated through cut/paste).
> 
> +    int inCustomCommand;
> 
> The name sounds like a bool but it's a counter of some sort (a
> level?), can it be improved?
> 
> +        kDebug(32500) << "commandStack is now: " << commandStack.count();
> +    }
> +    else if (addNewCommand) {
> 
> else's in this construct on same line as {.
> 
> +                textData =
> qobject_cast<KoTextShapeDataBase*>(oldParent->userData());
> +            } else  if (container) {
> 
> s/  / /
> 
> Great work!
> Elvis
> _______________________________________________
> calligra-devel mailing list
> calligra-devel at kde.org
> https://mail.kde.org/mailman/listinfo/calligra-devel
Implemented, thanks