release plan and request for merge
Elvis Stansvik
elvstone at gmail.com
Thu Feb 23 10:57:56 GMT 2012
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
More information about the calligra-devel
mailing list