The files have been formatted according to the KDE/Qt conventions.
The signals in the consructor of the Autostart class now use the Qt5 syntax.
The loops for the programs/scripts in the load method (Autostart class) are using the C++ 11 for loops
with const references instead of an index based loop.
Details
Details
- Reviewers
mlaurent davidedmundson meven
Should work as before
Diff Detail
Diff Detail
- Repository
- R119 Plasma Desktop
- Branch
- master
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 21654 Build 21672: arc lint + arc unit
| kcms/autostart/addscriptdialog.cpp | ||
|---|---|---|
| 39 | you can remove this line and adding (this) to new QVBoxLayout | |
| kcms/autostart/advanceddialog.cpp | ||
| 39 | you can remove this line as we have = new QVBoxLayout(this); | |
| kcms/autostart/autostart.cpp | ||
| 198 | you must use qAsConst(...) > const auto var = autostartdir.entryInfoList();for (const ... : var) { | |
| 244 | same here | |
| 408 | if (!widget...) { | |
Comment Actions
This kcms is going through some refactoring in D26934 , not the greatest time for a mostly formatting PR.
This will introduce a lot of conflict.
Comment Actions
Hello,
I already wondered how this will be handled.
Should I abandon this revision now and maybe create a new one with the relevant changes once your request has been merged ?
And is there any way I could have known that this kcm is going to be refactored ?
And a general question: When are accepted patches merged ? For instance your patch https://phabricator.kde.org/D26478 is very simple and has been accepted for a few weeks now, but has not been merged.
Comment Actions
I think that's the easiest way. Hopefully you use a script for this formatting and won't loose too much time.
And is there any way I could have known that this kcm is going to be refactored ?
Unfortunately there is no easy way but follow a little the activity on phabricator which is how I found your review.
I wish we had some notification/information when we touch a file or folder that has currently running reviews.
I don't know any forge platform that does this kind of thing.
And a general question: When are accepted patches merged ?
It is variable.
The smallest the more inconsequential and more consensual, the more tested and the more easy to test, the more people and experienced people have accepted it, the fastest it should land.
On the opposite, the more code you touch or on the more sensible things should make you should be patient and give reasonable time for other reviewers to comment/accept/or require change, especially experienced reviewers or maintainers.
You will then probably need to remind reviewers to have a look. We have sortof a reviewer shortage.
Doc does not specify this :/ https://community.kde.org/Infrastructure/Phabricator#Step_3:_Land_your_diff
For instance your patch https://phabricator.kde.org/D26478 is very simple and has been accepted for a few weeks now, but has not been merged.
This one I forgot about, I thought I landed it.
A tip, use https://phabricator.kde.org/differential/query/active/ to have a quick overview on your reviews.
Comment Actions
Yes, I have used an automatic formatter.
And in general about formatting: When I commit a patch which contains code changes, should I format the files (even the code that I don't directly edit) or would it be easier for the reviewers if I commit this in a separate patch ?
Or should I not reformat the files, to not stress the reviewers/maintainers ?
And what would be a good place to ask about the code of a project or discuss some ideas (on both Phabricator and Gitlab).
Thank you very much :-)