autotests/testkwaylandbackend.cpp
88	Why `true`?
autotests/testlog.cpp
52	I thought when concatenating the advantage of `QStringLiteral` don't cut it since you already have a string so should be `QLatin1String` instead?
autotests/testqscreenbackend.cpp
62	Not from local 8 bit?
174–175	Could use initializer list
autotests/testscreenconfig.cpp
104	`QLatin1String`?
backends/fake/parser.cpp
187–189	`QLatin1String`?
backends/xrandr1.1/xrandr11.cpp
174	`QLatin1Char`?

gladhorn marked 3 inline comments as done.Jul 12 2018, 6:47 AM

gladhorn added inline comments.

autotests/testkwaylandbackend.cpp
88	Indeed, that makes no sense.
autotests/testlog.cpp
52	You are right.
autotests/testqscreenbackend.cpp
174–175	I feel the change is big enough as it is :)
backends/fake/parser.cpp
187–189	My understanding is that since it's a QMap<QString, ..> all things will be converted to QString in any case.

Fixed some of Kai's comments, some minor things are still to do.

Harbormaster completed remote builds in B774: Diff 37616.Jul 12 2018, 6:48 AM

gladhorn added inline comments.Jul 12 2018, 6:50 AM

tests/kwayland/waylandtestserver.cpp
39	Concatenation is still faster with latin1string.

anthonyfieroni added a subscriber: anthonyfieroni.Jul 12 2018, 6:52 AM

anthonyfieroni added inline comments.

backends/fake/parser.cpp
145	It should be QLatin1String on contains?

zzag added a subscriber: zzag.Jul 12 2018, 7:08 AM

zzag added inline comments.

backends/fake/parser.cpp
145	http://doc.qt.io/qt-5/qbytearray.html#contains-1

I think micro-optimizing the tests further is simply not worth anyone's time. I'd propose taking this in (and if you spot more things, do them on top of this change).

I think micro-optimizing the tests further is simply not worth anyone's time

Me too.
Previous +2 applies

Closed by commit R110:973bef8349d2: Clean up string casts (authored by gladhorn). · Explain WhyJul 12 2018, 9:19 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			CMakeLists.txt (9 lines)
M			autotests/testbackendloader.cpp (2 lines)
M			autotests/testkwaylandbackend.cpp (4 lines)
M			autotests/testkwaylandconfig.cpp (4 lines)
M			autotests/testkwaylanddpms.cpp (2 lines)
M			autotests/testlog.cpp (6 lines)
M			autotests/testqscreenbackend.cpp (27 lines)
M			autotests/testscreenconfig.cpp (10 lines)
M			backends/fake/fake.cpp (8 lines)
M			backends/fake/parser.cpp (73 lines)
M			backends/kwayland/waylandoutput.cpp (2 lines)
M			backends/qscreen/qscreenbackend.cpp (4 lines)
M			backends/qscreen/qscreenoutput.cpp (4 lines)
M			backends/utils.cpp (30 lines)
M			backends/xcbeventlistener.cpp (22 lines)
M			backends/xrandr/xrandroutput.cpp (4 lines)
M			backends/xrandr1.1/xrandr11.cpp (2 lines)
M			src/backendmanager.cpp (9 lines)
M			src/configoperation.cpp (2 lines)
M			src/doctor/doctor.cpp (44 lines)
M			src/doctor/main.cpp (20 lines)
M			src/edid.cpp (6 lines)
M			src/log.cpp (20 lines)
M			src/output.cpp (2 lines)
M			tests/kwayland/main.cpp (6 lines)
M			tests/kwayland/waylandconfigreader.cpp (48 lines)
M			tests/kwayland/waylandtestserver.cpp (18 lines)
M			tests/testplugandplay.cpp (2 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	37599	bd4fa7d		Jul 11 2018, 9:48 PM	★	★
Diff 2	37616	bd4fa7d	Fixed some of Kai's comments, some minor things are still to do.	Jul 12 2018, 6:48 AM	★	★
Diff 3	37628	967f21a	R110:973bef8349d2acc88badafdb6b5437a19162f026	Jul 12 2018, 9:19 AM	★	★