Replacing connect() lines with the new signal/slot syntax in Qt5. Details: https://wiki.qt.io/New_Signal_Slot_Syntax.
There many of them. This is just the fist part, more will follow.
Details
- Reviewers
abika nmel - Group Reviewers
Krusader - Commits
- R167:65066df6f57d: Replaced old connect() with QT5 style. Part 1
Diff Detail
- Repository
- R167 Krusader
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
I only skim-read the changes, looks fine.
And Krusader seems to work with the changes.
Thanks for making code more prettier!
Cutting bigger changes into multiple Diffs and commits is totally fine, btw.
krusader/GUI/kcmdline.h | ||
---|---|---|
73 โ | (On Diff #43654) | unwanted change |
Is it approved and will be landed, or I have to send another arc diff with these 3 adjustments?
Thanks for reply
Is it approved and will be landed, or I have to send another arc diff with these 3 adjustments?
You have to make the changes requested by Nikita.
For minor tweaks like this we usually approve and let author incorporate the fixes before pushing. Just to reduce turnaround time. Of course, if someone else pushes (or lands) the change, the author should send updated diff first.
Miroslav, are you going to push this change to the repo or you want us to do it? I think it's worth getting the permission as you're going to do several consecutive changes on this refactoring and, hopefully, other improvements. Please read our commit guidelines before pushing.
I can push it, I might need some help, what is the best way :
Merge my branch to master and then push the master? Is there some thing else needed?
I can push it, I might need some help:
Is it enough to merge my branch to master, add code review link (= Differential revision https://phabricator.kde.org/D16223 ??) and push the master?
Or is there something more?
Thank you,
Miro
Since this is a single change and not multiple related changes, I would squash your branch into a single commit, rebase on top of the master, add the CR link and push it. Thanks!
OK, so will you provide me the necessary permissions for that? I suppose, it is this repository: git@git.kde.org:krusader.git
Thank you !
OK, so will you provide me the necessary permissions for that? I suppose, it is this repository: git@git.kde.org:krusader.git
Sure.
Can you follow these instructions?
https://community.kde.org/Infrastructure/Get_a_Developer_Account
BTW, many thanks for your work!