Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?

http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
Do not use QStringLiteral for empty strings


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 12:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings

afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
unit-tested, but there might be some code path I missed.
can you paste the actual warning it emitted, and also the .cpp file+line number 
?


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/
---

Review request for KDE Frameworks and Aleix Pol Gonzalez.


Repository: kitemviews


Description
---

Fix Clazy warnings, except:
/src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
qDeleteAll(d->widgetInIndex.keys());


Diffs
-

  autotests/klistwidgetsearchlinetest.cpp b86d62c 
  src/kcategorizedview.cpp 185c24c 
  src/ktreewidgetsearchline.cpp b721c58 
  tests/kcategorizedviewtest.cpp 16dac9a 
  tests/ktreewidgetsearchlinetest.cpp adaf246 
  tests/kwidgetitemdelegatetest.cpp d4bba7a 

Diff: https://git.reviewboard.kde.org/r/126737/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 1:03 p.m., Sergio Martins wrote:
> > Ship It!

Can you push it in two different commits ? One for the QStringLiteral change 
and another for the isEmpty()


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---

Ship it!


Ship It!

- Sergio Martins


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


Looks good, I'll add Sergio, he maybe can explain what the warning meant in the 
first place.

Andrey, Are you sure you fixed all the warnings? I found some missing, for 
example:
`/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1: 
warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
[-Wclazy-missing-qobject]`

- Aleix Pol Gonzalez


On Jan. 14, 2016, 10:49 a.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 10:49 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 1:03 p.m., Sergio Martins wrote:
> > Ship It!
> 
> Sergio Martins wrote:
> Can you push it in two different commits ? One for the QStringLiteral 
> change and another for the isEmpty()

Regarding the qDeleteAll(), the correct way is 
qDeleteAll(d->widgetInIndex.keyBegin(), d->widgetInIndex.keyEnd()), but that 
only works since Qt 5.6, so ignore this for now


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`

Strangely, I had not noticed the warning in the log . I get to the PC - see the 
log again.


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.

Some warnings , such as using qstringlitetal to initialize a blank line , I 
thought it wrong. Article Qt Weekly says that empty lines use the 
qstringliteral is not effective.


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^
> 
> Andrey Cygankov wrote:
> I do not have this warning, which showed Aleix Pol.
> That log with warnings, the analyzer built from anongit on January 7
> https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0

When correcting warnings that indicated Aleix Pol(Insert Q_OBJECT macro in 
kwidgetitemdelegatepool.cpp, which contains class definition), assembly error: 
"AUTOGEN: error: 
/home/andrey/dev/projects/kitemviews/src/kwidgetitemdelegatepool.cpp: The file 
contains a Q_OBJECT macro, but does not include "kwidgetitemdelegatepool.moc" !"


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 1:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^
> 
> Andrey Cygankov wrote:
> I do not have this warning, which showed Aleix Pol.
> That log with warnings, the analyzer built from anongit on January 7
> https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0
> 
> Andrey Cygankov wrote:
> When correcting warnings that indicated Aleix Pol(Insert Q_OBJECT macro 
> in kwidgetitemdelegatepool.cpp, which contains class definition), assembly 
> error: "AUTOGEN: error: 
> /home/andrey/dev/projects/kitemviews/src/kwidgetitemdelegatepool.cpp: The 
> file contains a Q_OBJECT macro, but does not include 
> "kwidgetitemdelegatepool.moc" !"

In any case, you should pass QString() there, instead of "".


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 1:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 1:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Aleix Pol Gonzalez


> On Jan. 14, 2016, 2:03 p.m., Sergio Martins wrote:
> > Ship It!
> 
> Sergio Martins wrote:
> Can you push it in two different commits ? One for the QStringLiteral 
> change and another for the isEmpty()
> 
> Sergio Martins wrote:
> Regarding the qDeleteAll(), the correct way is 
> qDeleteAll(d->widgetInIndex.keyBegin(), d->widgetInIndex.keyEnd()), but that 
> only works since Qt 5.6, so ignore this for now
> 
> Andrey Cygankov wrote:
> I do not have rights to commit.
> I think that it is not necessary to divide the RR into two parts. But I 
> have divided into two different diff, to make it convenient, here they are:
> 
> https://www.dropbox.com/s/tg1bhg71myv5nqb/kitemviews-clazyfix-tocommit.patch?dl=0
> 
> https://www.dropbox.com/s/j9chp5rx2cjkeay/kitemviews-isemptyfix-tocommit.patch?dl=0

While at it, can you request the commits right?
https://techbase.kde.org/Contribute/Get_a_Contributor_Account


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Jan. 14, 2016, 11:40 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 11:40 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 1:03 п.п., Sergio Martins wrote:
> > Ship It!
> 
> Sergio Martins wrote:
> Can you push it in two different commits ? One for the QStringLiteral 
> change and another for the isEmpty()
> 
> Sergio Martins wrote:
> Regarding the qDeleteAll(), the correct way is 
> qDeleteAll(d->widgetInIndex.keyBegin(), d->widgetInIndex.keyEnd()), but that 
> only works since Qt 5.6, so ignore this for now

I do not have rights to commit.
I think that it is not necessary to divide the RR into two parts. But I have 
divided into two different diff, to make it convenient, here they are:
https://www.dropbox.com/s/tg1bhg71myv5nqb/kitemviews-clazyfix-tocommit.patch?dl=0
https://www.dropbox.com/s/j9chp5rx2cjkeay/kitemviews-isemptyfix-tocommit.patch?dl=0


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91076
---


On Янв. 14, 2016, 10:40 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 10:40 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 12:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^
> 
> Andrey Cygankov wrote:
> I do not have this warning, which showed Aleix Pol.
> That log with warnings, the analyzer built from anongit on January 7
> https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0
> 
> Andrey Cygankov wrote:
> When correcting warnings that indicated Aleix Pol(Insert Q_OBJECT macro 
> in kwidgetitemdelegatepool.cpp, which contains class definition), assembly 
> error: "AUTOGEN: error: 
> /home/andrey/dev/projects/kitemviews/src/kwidgetitemdelegatepool.cpp: The 
> file contains a Q_OBJECT macro, but does not include 
> "kwidgetitemdelegatepool.moc" !"
> 
> Aleix Pol Gonzalez wrote:
> In any case, you should pass QString() there, instead of "".

Those QString warnings seems correct to me. "" calls the QString(const char*) 
ctor, which we don't want.
Also note that clazy didn't suggest using QStringLiteral for those cases.
For the moc problem, you can include kwidgetitemdelegatepool.moc at the end of 
the .cpp file


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 10:40 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 10:40 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/
---

(Updated Jan. 14, 2016, 11:21 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.


Changes
---

Submitted with commit 69b721e73b421c1950ca32f1e774471060f2d4fe by Aleix Pol on 
behalf of Andrey Cygankov to branch master.


Repository: kitemviews


Description
---

Fix Clazy warnings, except:
/src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
qDeleteAll(d->widgetInIndex.keys());


Diffs
-

  autotests/klistwidgetsearchlinetest.cpp b86d62c 
  src/kcategorizedview.cpp 185c24c 
  src/ktreewidgetsearchline.cpp b721c58 
  tests/kcategorizedviewtest.cpp 16dac9a 
  tests/ktreewidgetsearchlinetest.cpp adaf246 
  tests/kwidgetitemdelegatetest.cpp d4bba7a 

Diff: https://git.reviewboard.kde.org/r/126737/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/
---

(Updated Янв. 14, 2016, 10:40 п.п.)


Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.


Repository: kitemviews


Description
---

Fix Clazy warnings, except:
/src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
qDeleteAll(d->widgetInIndex.keys());


Diffs (updated)
-

  autotests/klistwidgetsearchlinetest.cpp b86d62c 
  src/kcategorizedview.cpp 185c24c 
  src/ktreewidgetsearchline.cpp b721c58 
  tests/kcategorizedviewtest.cpp 16dac9a 
  tests/ktreewidgetsearchlinetest.cpp adaf246 
  tests/kwidgetitemdelegatetest.cpp d4bba7a 

Diff: https://git.reviewboard.kde.org/r/126737/diff/


Testing
---

Build without errors.
Tests passed.


Thanks,

Andrey Cygankov

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?
> 
> Andrey Cygankov wrote:
> /tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << 
> "" << "Shop");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << 
> "" << "Ground");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
> "Market");
> ^
> /tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) 
> being called [-Wclazy-qstring-uneeded-heap-allocations]
> new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" 
> << "Farmer");
>^

I do not have this warning, which showed Aleix Pol.
That log with warnings, the analyzer built from anongit on January 7
https://www.dropbox.com/s/dtbsge2j8waojif/kit.log?dl=0


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Andrey Cygankov


> On Янв. 14, 2016, 12:35 п.п., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.
> 
> Sergio Martins wrote:
> what do you mean by blank line, and can you link to that Qt weekly 
> article ?
> 
> Andrey Cygankov wrote:
> http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/
> Do not use QStringLiteral for empty strings
> 
> Sergio Martins wrote:
> afaik clazy doesn't suggest QStringLiteral for empty QStrings, it's even 
> unit-tested, but there might be some code path I missed.
> can you paste the actual warning it emitted, and also the .cpp file+line 
> number ?

/tests/ktreewidgetsearchlinetest.cpp:78:73: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Pickled"  << "$4.00" << "" << 
"Shop");
^
/tests/ktreewidgetsearchlinetest.cpp:77:73: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Decaying" << "$0.50" << "" << 
"Ground");
^
/tests/ktreewidgetsearchlinetest.cpp:76:69: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Ripe" << "$8.00" << "" << 
"Market");
^
/tests/ktreewidgetsearchlinetest.cpp:75:72: warning: QString(const char*) being 
called [-Wclazy-qstring-uneeded-heap-allocations]
new QTreeWidgetItem(item, QStringList() << "Growing" << "$2.00" << "" << 
"Farmer");
   ^


- Andrey


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Янв. 14, 2016, 12:35 п.п., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Янв. 14, 2016, 12:35 п.п.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 126737: Fix most Clazy warnings in KItemViews.

2016-01-14 Thread Sergio Martins


> On Jan. 14, 2016, 12:35 p.m., Aleix Pol Gonzalez wrote:
> > Looks good, I'll add Sergio, he maybe can explain what the warning meant in 
> > the first place.
> > 
> > Andrey, Are you sure you fixed all the warnings? I found some missing, for 
> > example:
> > `/home/kde-devel/frameworks/kitemviews/src/kwidgetitemdelegatepool.cpp:48:1:
> >  warning: KWidgetItemDelegateEventListener is missing a Q_OBJECT macro 
> > [-Wclazy-missing-qobject]`
> 
> Andrey Cygankov wrote:
> Strangely, I had not noticed the warning in the log . I get to the PC - 
> see the log again.
> 
> Andrey Cygankov wrote:
> Some warnings , such as using qstringlitetal to initialize a blank line , 
> I thought it wrong. Article Qt Weekly says that empty lines use the 
> qstringliteral is not effective.

what do you mean by blank line, and can you link to that Qt weekly article ?


- Sergio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126737/#review91067
---


On Jan. 14, 2016, 12:35 p.m., Andrey Cygankov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126737/
> ---
> 
> (Updated Jan. 14, 2016, 12:35 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: kitemviews
> 
> 
> Description
> ---
> 
> Fix Clazy warnings, except:
> /src/kwidgetitemdelegatepool.cpp:153:5: warning: Calling qDeleteAll with 
> QHash::keys, call qDeleteAll on the container itself [-Wclazy-qdeleteall]
> qDeleteAll(d->widgetInIndex.keys());
> 
> 
> Diffs
> -
> 
>   autotests/klistwidgetsearchlinetest.cpp b86d62c 
>   src/kcategorizedview.cpp 185c24c 
>   src/ktreewidgetsearchline.cpp b721c58 
>   tests/kcategorizedviewtest.cpp 16dac9a 
>   tests/ktreewidgetsearchlinetest.cpp adaf246 
>   tests/kwidgetitemdelegatetest.cpp d4bba7a 
> 
> Diff: https://git.reviewboard.kde.org/r/126737/diff/
> 
> 
> Testing
> ---
> 
> Build without errors.
> Tests passed.
> 
> 
> Thanks,
> 
> Andrey Cygankov
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel