I used CLion inspection to hunt all unused #include
Details
Details
- Reviewers
elvisangelaccio markg - Group Reviewers
Dolphin - Commits
- R318:48b58f830a58: Remove unused #include
R318:848abc592216: Remove unused #include
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Branch
- remove-unused-include
- Lint
No Linters Available - Unit
No Unit Test Coverage
| src/views/dolphinremoteencoding.cpp | ||
|---|---|---|
| 36 | Please remove the KI18n prefix. | |
Comment Actions
This is really great! Nobody every cleans up unused includes because it's tedious i suppose.
But having this done is just awesome, good work!
Feel free to push after fixing the include from @elvisangelaccio and me.
Small heads up, i did just compile test dolphin with this change and it does work on linux, but be prepared for possible compile errors on windows or other platforms. So please do keep en eye open for jenkins mails about failing builds after this commit.
There might be nothing wrong, you will know after you push :)
| src/kitemviews/kitemlistcontainer.cpp | ||
|---|---|---|
| 33 | Remove the module name (QtWidgets). | |
Comment Actions
Can somebody land it for me? I have some problems:
$ arc land Landing current branch 'remove-unused-includes'. TARGET Landing onto "master", the default target under git. REMOTE Using remote "origin", the default remote under git. FETCH Fetching origin/master... Usage Exception: There are no commits on "remove-unused-includes" which are not already present on the target.
Comment Actions
It looks like you already pushed it? https://cgit.kde.org/dolphin.git/commit/?id=848abc5922167a467bb73107ee6b72e9af3c8317
Comment Actions
Well. I don't know how did that happen. Maybe I was unconscious.
In other words, how to close this diff?
Comment Actions
I don't know.
It's also weird that phabricator didn't pick it up as you do have the differential line in there.
Perhaps @bcooksley can help us out?
Comment Actions
It appears the commit hasn't been processed by Phabricator yet as per 848abc5922167a467bb73107ee6b72e9af3c8317
Looking closer at the backend it seems this commit has something in it which makes Phabricator quite unhappy, i'll have to look further into that and probably liase with upstream to get it fixed.
Comment Actions
-1 this is totally unrelated. For future reference, i see new functions being added (setFocus and hasFocus and the addition of an action.
Comment Actions
My bad. arc somehow updated a wrong diff from unrelated branch. Fixed it. As a bonus, I sorted all includes. I know that it's not important, but I cannot stand unsorted includes.
Comment Actions
Now it makes more sense.
Careful with arc ;)
I see what you did in this change, it does make sense but is becoming difficult te review as a consequence. As now it's difficult to see which includes are re-organizes, which are removed and which are perhaps both...
It does look a lot more organized now :)
I looked through all of it and it all seems fine to me.
I'd say: ship it! But keep it at this with the include refactoring please.
Note: while you're at it, all the kitemviews/... includes should be within quotes (it's locally to dolphin, it's not an external library). You've changed a couple to quotes (good!) but there are quite a few more with < and >.
I didn't mention it in the specific files as it really is fine with me as is, but if you want to change it, feel free :)