D16721: Use Krita toolbar in Karbon

[Anthony Fieroni]

anthonyfieroni added a comment.


  > I'm adding Krita as reviewer, because Karbon is unmaintained and I hope someone at Krita as some experience with the toolbox
  
  I'm the maintainer of Karbon, for now.
  
  Please revert foreach -> Q_FOREACH changes, it should be done in separate patch. Clean dead / commented code, white-space changes, don't remove QObject macro, unless you have a good reason for that. Override is good feature but in separate patch for old code, for new one it's good.
  
  Back-porting patches from Krita are welcome but in accepted condition to other part of the project.

INLINE COMMENTS

> KoToolBoxDocker.cpp:39
> +
> +    QLabel *w = new QLabel(" ", this);
> +    w->setFrameShape(QFrame::StyledPanel);

Why empty or white-space label?

> KoToolBoxLayout_p.h:51
> +        if (m_priorities.values().contains(priority)) {
> +            qWarning() << "Button" << button << "has a conflicting priority";
> +        }

It should be qCWarning, with category or remove it.

> KoToolBoxLayout_p.h:86
>          int x = 0;
> -        int y = 0;
> -        const QSize &size = buttonSize();
> +        int y = 0; const QSize &size = buttonSize();
>          if (m_orientation == Qt::Vertical) {

Revert.

> KoToolBoxLayout_p.h:236-237
>  
> +
> +protected:
>  private:

Revert

> KoKineticScroller.cpp:26
> +
> +QScroller* KoKineticScroller::createPreconfiguredScroller(QAbstractScrollArea *scrollArea) {
> +    KConfigGroup config = KSharedConfig::openConfig()->group("");

{ on new line.

> KoKineticScroller.cpp:89
> +
> +QScroller::ScrollerGestureType KoKineticScroller::getConfiguredGestureType() {
> +    KConfigGroup config = KSharedConfig::openConfig()->group("");

{ on new line.

REPOSITORY
  R8 Calligra

REVISION DETAIL
  https://phabricator.kde.org/D16721

To: ognarb, #krita, #calligra:_3.0
Cc: anthonyfieroni, Calligra-Devel-list, dcaliste, cochise, vandenoever
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20181107/326a1f76/attachment.htm>