romangg added a comment.
I'm not sure about the renaming. the lock... implies that the rotation stays the way it is at the moment (what it does). But I'm fine either way. These are normally two separate commits, one refactor and one feature addition. But since it's only a small change and Phabricator sucks let's not bitch too much about it. @bshah: ok with the change? INLINE COMMENTS > config.cpp:104 > + } > + ret &= m_control->getAutoRotate(output); > + } Should we check if all or only one panel display are set to auto rotate. Granted I'm not aware of a device with multiple panel displays which runs KScreen but maybe you had a reason for using && here? Just want to hear a rational if you had one. If you didn't and just picked && by random it's also fine. REPOSITORY R104 KScreen REVISION DETAIL https://phabricator.kde.org/D28773 To: z3ntu, #plasma, bshah Cc: romangg, nicolasfella, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart