D17028: Loader: Avoid Q_FOREACH

2018-11-20 Thread Sergio Martins 
smartins added inline comments.

INLINE COMMENTS

> loader.cpp:273
>  QStringList allLocalizedDictionaries;
> -Q_FOREACH (const QString , languages()) {
> +for (const QString  : languages()) {
>  allLocalizedDictionaries.append(languageNameForCode(langCode));

will languages() detach ?

REPOSITORY
  R246 Sonnet

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

To: loh.tar, davidedmundson
Cc: smartins, kde-frameworks-devel, michaelh, ngraham, bruns

[Differential] [Commented On] D4584: KDirWatch: replace QList by std::vector to save on new/delete.

2017-02-15 Thread Sergio Martins 
smartins added inline comments.

INLINE COMMENTS

> dfaure wrote in kdirwatch.cpp:1379
> Detaches? a std::vector?

doh!

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, aacid, mpyne, mwolff
Cc: smartins, mwolff, #frameworks

[Differential] [Changed Subscribers] D4584: KDirWatch: replace QList by std::vector to save on new/delete.

2017-02-15 Thread Sergio Martins 
smartins added inline comments.

INLINE COMMENTS

> kdirwatch.cpp:1379
>  
> -Q_FOREACH (Client *c, e->m_clients) {
> -if (c->instance == nullptr || c->count == 0) {
> +for (Client  : e->m_clients) {
> +if (c.instance == nullptr || c.count == 0) {

can m_clients be shared ? If yes, htis detaches

REPOSITORY
  R244 KCoreAddons

BRANCH
  master

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: dfaure, aacid, mpyne, mwolff
Cc: smartins, mwolff, #frameworks

Re: Review Request 127211: Properly use static QMaps

2016-02-29 Thread Sergio Martins 


> On Feb. 29, 2016, 10:36 a.m., Milian Wolff wrote:
> > even better would be to use
> > 
> > static const QHash map = {
> > {QStringLiteral(...), value}, // NOTE: don't use QLatin1String here!
> > ...
> > };
> > return map[mapCommand];

QStringLiteral here would break msvc2012, afaik


- Sergio


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


On Feb. 28, 2016, 9:52 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127211/
> ---
> 
> (Updated Feb. 28, 2016, 9:52 p.m.)
> 
> 
> Review request for Kate and KDE Frameworks.
> 
> 
> Repository: ktexteditor
> 
> 
> Description
> ---
> 
> Only initialize them once.
> Since they're not const, use ::value rather than ::operator[], because the 
> latter will provide a default-constructed object and add it to the map.
> 
> 
> Diffs
> -
> 
>   src/vimode/cmds.cpp 7804af4 
> 
> Diff: https://git.reviewboard.kde.org/r/127211/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

___
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

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 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 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 qDeleteA

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

Re: Review Request 126429: Fixed all Clazy warnings level 1 and level 2

2015-12-23 Thread Sergio Martins 


> On Dec. 23, 2015, 2:49 p.m., Aleix Pol Gonzalez wrote:
> > src/k7zip.cpp, line 2029
> > <https://git.reviewboard.kde.org/r/126429/diff/1/?file=424823#file424823line2029>
> >
> > I wouldn't get this all in. Is that suggested by 
> > `-Wclazy-reserve-candidates`?
> 
> Artur Puzio wrote:
> Yes this is suggested by `-Wclazy-reserve-candidates`.

I think this one is not worth it, as it makes the code more complex.
Also note that QVector already has a growth strategy, so it's not terrible.


- Sergio


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


On Dec. 23, 2015, 2:49 p.m., Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126429/
> ---
> 
> (Updated Dec. 23, 2015, 2:49 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: karchive
> 
> 
> Description
> ---
> 
> Fixed warning: Folder has dtor but not copy-ctor, copy-assignment 
> [-Wclazy-rule-of-three], by adding Q_DISABLE_COPY(Folder)
> Fixed warning: Reserve candidate [-Wclazy-reserve-candidates], by 
> precalculateing QVector size.
> Fixed warning: unused variable [-Wunused-const-variable], by commenting 
> unused variables
> 
> 
> Diffs
> -
> 
>   src/k7zip.cpp 7b5e6a3 
> 
> Diff: https://git.reviewboard.kde.org/r/126429/diff/
> 
> 
> Testing
> ---
> 
> Automated tests pass.
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

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

Re: Review Request 126429: Fixed all Clazy warnings level 1 and level 2

2015-12-23 Thread Sergio Martins 

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


- Sergio Martins


On Dec. 23, 2015, 2:49 p.m., Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126429/
> ---
> 
> (Updated Dec. 23, 2015, 2:49 p.m.)
> 
> 
> Review request for KDE Frameworks, Aleix Pol Gonzalez and Sergio Martins.
> 
> 
> Repository: karchive
> 
> 
> Description
> ---
> 
> Fixed warning: Folder has dtor but not copy-ctor, copy-assignment 
> [-Wclazy-rule-of-three], by adding Q_DISABLE_COPY(Folder)
> Fixed warning: Reserve candidate [-Wclazy-reserve-candidates], by 
> precalculateing QVector size.
> Fixed warning: unused variable [-Wunused-const-variable], by commenting 
> unused variables
> 
> 
> Diffs
> -
> 
>   src/k7zip.cpp 7b5e6a3 
> 
> Diff: https://git.reviewboard.kde.org/r/126429/diff/
> 
> 
> Testing
> ---
> 
> Automated tests pass.
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

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

Re: Review Request 126350: Fixed all Clazy level 1 and level 2 warnings

2015-12-15 Thread Sergio Martins 

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



src/kuitmarkup.h (line 193)
<https://git.reviewboard.kde.org/r/126350/#comment61274>

extra whitespace


- Sergio Martins


On Dec. 15, 2015, 11:34 a.m., Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126350/
> ---
> 
> (Updated Dec. 15, 2015, 11:34 a.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Fixed `warning: KuitSetup has dtor, copy-ctor but not copy-assignment 
> [-Wclazy-rule-of-three]`
> Fixed `warning: Reserve candidate [-Wclazy-reserve-candidates]`
> Fixed `warning: Use QVariant::toFoo() instead of QVariant::value() 
> [-Wclazy-variant-sanitizer]`
> Fixed `warning: Use midRef() instead [-Wclazy-qstring-ref]`
> Fixed `warning: Pass small and trivially-copyable type by value (const class 
> QChar &) [-Wclazy-foreach]`
> Fixed `warning: QString::fromLatin1() being passed a literal 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(const char*) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(QLatin1String) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Changed `QLatin1String`-s to `char[]` when writteing to warning.
> Changed to `QStringLitera`-s in some lines, where Clazy isn't warning.
> 
> 
> Diffs
> -
> 
>   src/common_helpers.cpp dad7f84 
>   src/kcatalog.cpp 7711e9b 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/klocalizedstring.cpp 69950d2 
>   src/ktranscript.cpp 04dda66 
>   src/kuitmarkup.h d110ca3 
>   src/kuitmarkup.cpp 02b891a 
> 
> Diff: https://git.reviewboard.kde.org/r/126350/diff/
> 
> 
> Testing
> ---
> 
> Automated tests 1,3 and 4 passing.
> Test 2 fails on my system both: after changes and before.
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

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

Re: Review Request 126350: Fixed all Clazy level 1 and level 2 warnings

2015-12-14 Thread Sergio Martins 

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



src/klocalizedstring.cpp (line 389)
<https://git.reviewboard.kde.org/r/126350/#comment61246>

this will break the build with MSVC, it doesn't support QStringLiteral with 
multi-line literals.

(hint:
export CLAZY_EXTRA_OPTIONS="qstring-uneeded-heap-allocations-msvc-compat"
    )


- Sergio Martins


On Dec. 14, 2015, 6:46 p.m., Artur Puzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126350/
> ---
> 
> (Updated Dec. 14, 2015, 6:46 p.m.)
> 
> 
> Review request for KDE Frameworks and Aleix Pol Gonzalez.
> 
> 
> Repository: ki18n
> 
> 
> Description
> ---
> 
> Fixed `warning: KuitSetup has dtor, copy-ctor but not copy-assignment 
> [-Wclazy-rule-of-three]`
> Fixed `warning: Reserve candidate [-Wclazy-reserve-candidates]`
> Fixed `warning: Use QVariant::toFoo() instead of QVariant::value() 
> [-Wclazy-variant-sanitizer]`
> Fixed `warning: Use midRef() instead [-Wclazy-qstring-ref]`
> Fixed `warning: Pass small and trivially-copyable type by value (const class 
> QChar &) [-Wclazy-foreach]`
> Fixed `warning: QString::fromLatin1() being passed a literal 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(const char*) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> Fixed `warning: QString(QLatin1String) being called 
> [-Wclazy-qstring-uneeded-heap-allocations]`
> 
> 
> Diffs
> -
> 
>   src/kcatalog.cpp 7711e9b 
>   src/klocalizedcontext.cpp 3bc42dd 
>   src/common_helpers.cpp dad7f84 
>   src/klocalizedstring.cpp 69950d2 
>   src/ktranscript.cpp 04dda66 
>   src/kuitmarkup.h d110ca3 
>   src/kuitmarkup.cpp 02b891a 
> 
> Diff: https://git.reviewboard.kde.org/r/126350/diff/
> 
> 
> Testing
> ---
> 
> Automated tests 1,3 and 4 passing.
> Test 2 fails on my system both: after changes and before.
> 
> 
> Thanks,
> 
> Artur Puzio
> 
>

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

Re: Review Request 125106: Minor optimizations

2015-10-05 Thread Sergio Martins 


> On Sept. 8, 2015, 4:46 p.m., Milian Wolff wrote:
> > src/kconf_update/kconf_update.cpp, line 250
> > 
> >
> > QLatin1String
> 
> Albert Astals Cid wrote:
> Seems like a bug in clazy, sometimes it knows that it should use 
> QLatin1String but it seems it didn't in this case.

If you file a bug with test case I will fix these.


- Sergio


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


On Oct. 5, 2015, 1:16 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125106/
> ---
> 
> (Updated Oct. 5, 2015, 1:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Ran the clazy tool 
> (http://www.kdab.com/use-static-analysis-improve-performance/)
> 
> Mostly QStringLiteral/QLatin1String additions
> A few const & additions to non public methods
> 
> 
> Diffs
> -
> 
>   src/kconfig_compiler/kconfig_compiler.cpp 5c515f4 
>   src/kconf_update/kconfigutils.cpp 0fec63a 
>   src/kconf_update/kconf_update.cpp a1c98cb 
>   src/gui/kwindowconfig.cpp a32c6cc 
>   src/core/kdesktopfile.cpp 49f37c3 
>   src/core/ksharedconfig.cpp e059b87 
>   src/core/kconfigini.cpp 856b7b7 
>   src/core/kcoreconfigskeleton_p.h d098ef6 
>   src/core/kauthorized.cpp 4280524 
>   src/core/kconfig.cpp 4be9e6d 
>   src/core/kconfigbackend.cpp 67bdefa 
> 
> Diff: https://git.reviewboard.kde.org/r/125106/diff/
> 
> 
> Testing
> ---
> 
> Compiles, test pass
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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