Nice findings, feel free to ignore the stylistic changes I commented, except the qDeleteAll one, and do unrelated further cleanup in a separate patch

backends/kwayland/waylandoutput.cpp
64	I think we typically use `const...()` instead of `c...()` but since this method is `const`, shouldn't be neccessary to begin with
backends/qscreen/qscreenconfig.cpp
47	`qDeleteAll(m_outputMap);`
62	This seems unused
src/output.cpp
101	`count()`
tests/kwayland/waylandconfigreader.cpp
117	We do a double lookup here, `contains()` and then `operator[]` afterwards, should be combined to a single `find()`

volkov added inline comments.Oct 4 2018, 2:55 PM

backends/kwayland/waylandoutput.cpp
64	Isn't it better to follow STL style? cbegin is also popular in KDE projects: https://lxr.kde.org/search?_filestring=&_string=cbegin

fixed

Harbormaster completed remote builds in B3491: Diff 42860.Oct 4 2018, 3:06 PM

broulik accepted this revision.Oct 5 2018, 7:13 AM

broulik added inline comments.

src/output.cpp
101	The previous code also compared the contents of both, or does your new loop below do that?

This revision is now accepted and ready to land.Oct 5 2018, 7:13 AM

volkov added inline comments.Oct 5 2018, 11:02 AM

src/output.cpp
101	Yes, the check is in the loop below: `after` and `before` are of the same sizes, and if `after` contains each key from `before`, then their keys are equal. So return false if a key can't be found in `after`.

volkov closed this revision.Oct 5 2018, 11:04 AM

Revision Contents
Changeset List

			Path	Packages
M			backends/kwayland/waylandoutput.cpp (8 lines)
M			backends/qscreen/qscreenconfig.cpp (10 lines)
M			src/output.cpp (13 lines)
M			tests/kwayland/waylandconfigreader.cpp (4 lines)
M			tests/kwayland/waylandtestserver.cpp (5 lines)