D21568: Dock for Hypothesis Testing

Stefan Gerlach noreply at phabricator.kde.org
Wed Jun 5 20:28:00 BST 2019


sgerlach added a comment.


  First comments on code.

INLINE COMMENTS

> CMakeLists.txt:42
>  	${KDEFRONTEND_DIR}/datasources/MQTTErrorWidget.cpp
> -    ${KDEFRONTEND_DIR}/datasources/MQTTSubscriptionWidget.cpp
> +        ${KDEFRONTEND_DIR}/datasources/MQTTSubscriptionWidget.cpp
>  	${KDEFRONTEND_DIR}/datasources/JsonOptionsWidget.cpp

did you accidentally changed the indentation here?

> HypothesisTest.cpp:6
> +    --------------------------------------------------------------------
> +    Copyright            : (C) 2019 Alexander Semke(alexander.semke at web.de)
> +

Is this line correct?

> HypothesisTest.cpp:35
> +#include "backend/lib/macros.h"
> +#include "QDebug"
> +

including QDebug should not be necessary if you use only our macro DEBUG()

> HypothesisTestPrivate.h:6
> +    --------------------------------------------------------------------
> +    Copyright            : (C) 2019 by Alexander Semke (alexander.semke at web.de)
> +

s.a.

> SpreadsheetView.h:133
>  	QAction* action_pivot_table;
> -    QAction* action_do_ttest;
> +    QAction* action_do_htest;
>  

"htest" ist ambiguous. "hypothesis_test" ist clearer to me

> SpreadsheetView.h:210
>  	void createPivotTable();
> -    void doTTest();
> +    void doHTest();
>  	void sortSpreadsheet();

doHypothesisTest()

> MainWin.cpp:327
>  
> +    m_newHypothesisTestAction = new QAction(QIcon::fromTheme("labplot-spreadsheet-new"),i18n("Hypothesis Test"),this);
> +    m_newHypothesisTestAction->setWhatsThis(i18n("Creates windows for hypothesis testing"));

please insert a space before every "," to make it look better.

> HypothesisTestDock.cpp:6
> +    --------------------------------------------------------------------
> +    Copyright            : (C) 2019 by Alexander Semke (alexander.semke at web.de)
> +

s.a.

> HypothesisTestDock.cpp:46
> +#include <KMessageBox>
> +#include <QDebug>
> + /*!

s.a.

> HypothesisTestDock.h:6
> +    --------------------------------------------------------------------
> +    Copyright            : (C) 2019 by Alexander Semke (alexander.semke at web.de)
> +

s.a.

> HypothesisTestView.cpp:6
> +    --------------------------------------------------------------------
> +    Copyright            : (C) 2019 by Alexander Semke (alexander.semke at web.de)
> +

s.a.

> HypothesisTestView.cpp:34
> +
> +#include <QInputDialog>
> +#include <QFile>

are all these header needed?

> HypothesisTestView.cpp:51
> +
> +#include <QDebug>
> +

s.a.

> HypothesisTestView.h:2
> +/***************************************************************************
> +    File                 : PivotTableView.h
> +    Project              : LabPlot

not true

> HypothesisTestView.h:6
> +    --------------------------------------------------------------------
> +    Copyright            : (C) 2019 by Alexander Semke (alexander.semke at web.de)
> +

s.a.

> hypothesistestdock.cpp:1
> +#include "hypothesistestdock.h"
> +#include "ui_hypothesistestdock.h"

missing license header.

> hypothesistestdock.cpp:12
> +HypothesisTestDock::~HypothesisTestDock()
> +{
> +    delete ui;

no new line for "{"

> hypothesistestdock.h:1
> +#ifndef HYPOTHESISTESTDOCK_H
> +#define HYPOTHESISTESTDOCK_H

missing license header.

> ttest.diff:1
> +diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> +index 1c39d438f..9eea05497 100644

no need for reviewing diff files.

> ttest2.diff:1
> +diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> +index 1c39d438f..0d00a555b 100644

same

REPOSITORY
  R262 LabPlot

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

To: devanshuagarwal, asemke, sgerlach
Cc: kde-edu, #labplot, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190605/9b7ba5a7/attachment-0001.html>


More information about the kde-edu mailing list