The maximise test is moved and a simple test is added for smart
placement and placeCorner.
The class tries to make a framework to make it faster to add future
xdg_toplevel placement tests without having to copy too much
boilerplate.
zzag |
KWin |
The maximise test is moved and a simple test is added for smart
placement and placeCorner.
The class tries to make a framework to make it faster to add future
xdg_toplevel placement tests without having to copy too much
boilerplate.
Passes
Lint OK |
No Unit Test Coverage |
Buildable 13147 | |
Build 13165: arc lint + arc unit |
autotests/integration/placement.cpp | ||
---|---|---|
23–24 | Style: https://techbase.kde.org/Policies/Frameworks_Coding_Style#Includes (needs to be sorted) | |
42 | ||
61–66 | These two can be ordinary functions. Also, "doxygen" comments have to terminate with **/ | |
105–108 | I personally see nothing wrong with duplicating this piece of code in each test. | |
121 | Missing QVERIFY. | |
143 | Is it correct? It's null. | |
161–163 | A better name for the test: testPlaceZeroCornered. |
autotests/integration/placement.cpp | ||
---|---|---|
61–66 | Why would someone be generating doxygen of an internal class of an autotest? Yes they could be functions, but there's a semantic link and we're not making a public API or anything | |
105–108 | Till we change something... | |
121 | You can't QVERIFY outside of a test function. It has an empty return. If this was failing the shellClient test would be exploding, I think most people know to fix that first, which can leave this test to focus on placement. | |
143 | It was not. Thanks. |
autotests/integration/placement.cpp | ||
---|---|---|
30–37 | You forgot to sort these includes. | |
30–37 | Argh, please ignore it. It's sort of embarrassing.. | |
61–66 |
Perhaps I should have asked you this question. Initially, this method had a legit Doxygen comment with "wrong" terminator. | |
121 | Oops, sorry. | |
152 | Name of this variable is misleading. It leaves impression that this is a ShellClient. Perhaps don't use auto? |
autotests/integration/placement.cpp | ||
---|---|---|
61–66 | Yeah, I had it because I wanted to indicate the comment was describing what the function does, not commenting the code. But then as that went into a pointless discussion about doxygen, I didn't bother. | |
152 | Done, but we need to take this "auto" discussion to frameworks-devel and make an actual policy. |
autotests/integration/placement.cpp | ||
---|---|---|
152 | A bit radical claim: I suggest to prohibit auto in kwin because many people abuse it. We could use it with only a few things, like iterators. auto foo = bar(); is just wrong. I know that I did this too, but it's just wrong. Anyways, that's out of scope of this patch. |