Can you explain in more detail how an unreachable `break` after a `return` statement makes the code more readable?

In D15715#330743, @astippich wrote:

since I'm still unsure about those things: the q2t function was not declared static, but was never exported. It is still safe to remove, right?

Gentle ping. It is only a change of a unit label after all :)

since I'm still unsure about those things: the q2t function was not declared static, but was never exported. It is still safe to remove, right?

In D15634#330710, @anthonyfieroni wrote:

if you have suggest to watch some variable or container change?

• Tomaz's std::min suggetion

In D15711#330689, @aacid wrote:

I don't like qrc, are you saying that the only way kxmlgui works on MacOs is by not installing files?

This is weird since it seems to work just fine on Windows

In D15514#326187, @cfeck wrote:

I would even say 'after' looks better, because the bottom shadow isn't as strong as 'before'.

I run it with debug messages for a while, there is no leak in windows neither in stackingOrder. @zzag if you have suggest to watch some variable or container change? I can transform this review to some profiling changes, removing double lookups, const ref against copying of Pair and so on.

In D15694#330687, @rjvbb wrote:

As mentioned on another ticket, I don't like the idea of not putting breaks, no matter how clever the compiler and hard-coded choice of compiler options.

Looks good to me. A nice and simple way to drastically increase test coverage.

I don't like qrc, are you saying that the only way kxmlgui works on MacOs is by not installing files?

As mentioned on another ticket, I don't like the idea of not putting breaks, no matter how clever the compiler and hard-coded choice of compiler options; I don't think you gain anything from it, and it doesn't improve code readability either for me (esp. when switch cases aren't indented w.r.t the switch context, as in the last hunk in plugins/qmljs/codecompletion/context.cpp.
Making it obligatory to add a token confirming that fallthrough is intended is a good idea, though using a largish term in all-caps like Q_FALLTHROUGH also doesn't improve readibilty. And what about the most common type of fallthrough, are we really going to have to write something like the following?

FYI, I ported the Minimize Animation effect to JavaScript (zzag/port-minimizeanimation-to-js branch). Yet, there are several "obstacles":

• checking whether there is a full screen effect;
• reversing animations (easy to fix, but I think it will go only after ScriptedEffect is ported to QJSEngine).

Dear Luigi,
    I should inform you that I've been in touch with Andreas Sturnlechner (you were copied in) - who has pointed out that there is a KF5 version already. It's a pity I did not discover this earlier - but I am new to a number of the software tools, including GitHub.

• don't change test strings for now

Sure.

Remove comment in plugins/ghprovider/ghproviderwidget.cpp, as the fallthrough is probably intended, and add break in plugins/qmljs/codecompletion/context.cpp, as it is probably not intended. Now the code completion doesn't suggest variables of the same type as the last parameter in functions calls, which seems about right.

• again, please do not mix unrelated changes, just submit separate and documented review for them
• please follow a bit more the existing coding style

In D15585#328376, @rempt wrote:

In D15585#328367, @alvinhochun wrote:

This post [1] describes a way of building OpenSSL with mingw(-w64).

That's rather different from what's in the openssl readme itself :-(

• Removed unnecessary casts
• Altered dynamic to static cast
• Changed variables from uint to int type
• Removed casts

Doesn't KDevelop already add (install) a few mimetypes of its own? I think I mentioned before that I do have a .xml file for SMI, adding the x-objc++src definition and I presume that's why I am not seeing mimetype warnings about ObC (I'm seeing others though.)