Review Request 129390: KRecentFilesAction: remove unnecessary copy

[Elvis Angelaccio]

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

Review request for KDE Frameworks.


Repository: kconfigwidgets


Description
---

QUrl is implicitly shared, so the code does not do what the comment says.
Add also a test case to make sure that the crash described in the comment 
doesn't happen anymore (again, thanks to implicit sharing).


Diffs
-

  autotests/krecentfilesactiontest.h f09bf6349e5b98999373d762fdbc4fd3a1f8e8f4 
  autotests/krecentfilesactiontest.cpp 1ba100dd1a8b5295e28c436157283e4f493563cd 
  src/krecentfilesaction.cpp 7ffe2e7b75e028346af3215dada8e96ea940d65b 

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


Testing
---

All tests pass.


Thanks,

Elvis Angelaccio

Review Request 129389: KRecentFilesAction: improve addUrl() apidox

[Elvis Angelaccio]

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

Review request for KDE Frameworks.


Repository: kconfigwidgets


Description
---

If we add an url to the recent files list, the recentFilesAction will be 
enabled. This makes sense but can be unexpected (e.g. if one manually disabled 
the action), so add it to the apidox of the method.


Diffs
-

  src/krecentfilesaction.h c13c81f8e3c334e3efe229c8befcc50be37a75a0 

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


Testing
---


Thanks,

Elvis Angelaccio

Re: Review Request 129390: KRecentFilesAction: remove unnecessary copy

[Elvis Angelaccio]

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

(Updated Nov. 13, 2016, 11:45 a.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks.


Repository: kconfigwidgets


Description
---

QUrl is implicitly shared, so the code does not do what the comment says.
Add also a test case to make sure that the crash described in the comment 
doesn't happen anymore (again, thanks to implicit sharing).


Diffs
-

  autotests/krecentfilesactiontest.h f09bf6349e5b98999373d762fdbc4fd3a1f8e8f4 
  autotests/krecentfilesactiontest.cpp 1ba100dd1a8b5295e28c436157283e4f493563cd 
  src/krecentfilesaction.cpp 7ffe2e7b75e028346af3215dada8e96ea940d65b 

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


Testing
---

All tests pass.


Thanks,

Elvis Angelaccio

Re: Review Request 129390: KRecentFilesAction: remove unnecessary copy

[Anthony Fieroni]

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




src/krecentfilesaction.cpp (line 185)


removeAction(selectableActionGroup()->actions().first())->deleteLater()

This should work as expected, about me.


- Anthony Fieroni


On Ноев. 13, 2016, 1:45 след обяд, Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129390/
> ---
> 
> (Updated Ноев. 13, 2016, 1:45 след обяд)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> QUrl is implicitly shared, so the code does not do what the comment says.
> Add also a test case to make sure that the crash described in the comment 
> doesn't happen anymore (again, thanks to implicit sharing).
> 
> 
> Diffs
> -
> 
>   autotests/krecentfilesactiontest.h f09bf6349e5b98999373d762fdbc4fd3a1f8e8f4 
>   autotests/krecentfilesactiontest.cpp 
> 1ba100dd1a8b5295e28c436157283e4f493563cd 
>   src/krecentfilesaction.cpp 7ffe2e7b75e028346af3215dada8e96ea940d65b 
> 
> Diff: https://git.reviewboard.kde.org/r/129390/diff/
> 
> 
> Testing
> ---
> 
> All tests pass.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

Re: Review Request 129390: KRecentFilesAction: remove unnecessary copy

[Elvis Angelaccio]

> On Nov. 13, 2016, 12:22 p.m., Anthony Fieroni wrote:
> > src/krecentfilesaction.cpp, line 191
> > 
> >
> > removeAction(selectableActionGroup()->actions().first())->deleteLater()
> > 
> > This should work as expected, about me.

Doesn't make a difference. The crash happens when copying the QUrl, the QAction 
pointer is unrelated.


- Elvis


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


On Nov. 13, 2016, 11:45 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129390/
> ---
> 
> (Updated Nov. 13, 2016, 11:45 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> QUrl is implicitly shared, so the code does not do what the comment says.
> Add also a test case to make sure that the crash described in the comment 
> doesn't happen anymore (again, thanks to implicit sharing).
> 
> 
> Diffs
> -
> 
>   autotests/krecentfilesactiontest.h f09bf6349e5b98999373d762fdbc4fd3a1f8e8f4 
>   autotests/krecentfilesactiontest.cpp 
> 1ba100dd1a8b5295e28c436157283e4f493563cd 
>   src/krecentfilesaction.cpp 7ffe2e7b75e028346af3215dada8e96ea940d65b 
> 
> Diff: https://git.reviewboard.kde.org/r/129390/diff/
> 
> 
> Testing
> ---
> 
> All tests pass.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

Re: Review Request 129390: KRecentFilesAction: remove unnecessary copy

[Anthony Fieroni]

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




src/krecentfilesaction.cpp (line 99)


Yeah, you can't avoid copying, you can move it here
QUrl url = m_urls[action];
emit q->urlSelection(url);
But comment is needed again, because *strong* reference became weak


- Anthony Fieroni


On Ноев. 13, 2016, 1:45 след обяд, Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129390/
> ---
> 
> (Updated Ноев. 13, 2016, 1:45 след обяд)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> QUrl is implicitly shared, so the code does not do what the comment says.
> Add also a test case to make sure that the crash described in the comment 
> doesn't happen anymore (again, thanks to implicit sharing).
> 
> 
> Diffs
> -
> 
>   autotests/krecentfilesactiontest.h f09bf6349e5b98999373d762fdbc4fd3a1f8e8f4 
>   autotests/krecentfilesactiontest.cpp 
> 1ba100dd1a8b5295e28c436157283e4f493563cd 
>   src/krecentfilesaction.cpp 7ffe2e7b75e028346af3215dada8e96ea940d65b 
> 
> Diff: https://git.reviewboard.kde.org/r/129390/diff/
> 
> 
> Testing
> ---
> 
> All tests pass.
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

Re: Review Request 129389: KRecentFilesAction: improve addUrl() apidox

[Aleix Pol Gonzalez]

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



I'm not sure that it makes sense, or maybe it just reads weird.

- Aleix Pol Gonzalez


On Nov. 13, 2016, 12:33 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129389/
> ---
> 
> (Updated Nov. 13, 2016, 12:33 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kconfigwidgets
> 
> 
> Description
> ---
> 
> If we add an url to the recent files list, the recentFilesAction will be 
> enabled. This makes sense but can be unexpected (e.g. if one manually 
> disabled the action), so add it to the apidox of the method.
> 
> 
> Diffs
> -
> 
>   src/krecentfilesaction.h c13c81f8e3c334e3efe229c8befcc50be37a75a0 
> 
> Diff: https://git.reviewboard.kde.org/r/129389/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

Review Request 129394: [filenamesearch] Fix huge ram usage in kded module

[Anthony Fieroni]

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

Review request for KDE Frameworks and David Faure.


Repository: kio-extras


Description
---

Bug is introduced in https://git.reviewboard.kde.org/r/129297/
When is fixed new kio-extras realease is needed for 16.08 branch.


Diffs
-

  filenamesearch/kded/filenamesearchmodule.cpp 3f9f582 

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


Testing
---

No big ram usage but still not works as expected.
1. Perform search in Dolphin
2. Delete one result item
3. View must be update, but it's not


Thanks,

Anthony Fieroni