cleanup cmake executable preferences
ClosedPublic
Actions

Authored by rjvbb on Sep 20 2017, 3:09 PM.

Details

Reviewers

apol
volden

Group Reviewers

KDevelop

Summary

This cleans up the duplicate methods and operations that resulted from the recent merge of Morten's cmake executable preferences patch to the main branch with my own to the 5.2 branch.

This patch was made against the master branch.

I've standardised on currentCMakeExecutable() for consistency with existing conventions and stuck to my approach that centralises the fallback from project-setting to kdevelop-setting to hardcoded system default.

I've kept Morten's simpler use of a QString& argument in the setter, though.

Test Plan

builds and runs as intended (tested on the 5.2 branch).

Diff Detail

Lint

Lint Skipped

Unit

Unit Tests Skipped

rjvbb created this revision.Sep 20 2017, 3:09 PM

Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptSep 20 2017, 3:09 PM

I guess that it was my fault that the mess occurred anyway, hopefully it can now be cleaned up now matter which merge direction you use (the patch applies to both branches).

LGTM!

This revision is now accepted and ready to land.Sep 20 2017, 4:20 PM

@apol : Green light from you too?

It's very similar to the patch I pushed: 667891df61d4f90e20692d59b9bfc7c1ba8bd6b1

If there's some bits here that you are still missing, feel free to include them.

I had followed Morten in his use of a QString as the 2nd argument to setCurrentCMakeExecutable instead of the Path I used initially. Both conventions are used, I doubt one is significantly more expensive than the other and I have no actual preference in the matter.
As far as I'm concerned we can just as well leave things as they are.

Done, right? Closing.

Revision Contents
Changeset List

			Path	Packages
M			plugins/cmake/cmakeutils.h (12 lines)
M			plugins/cmake/cmakeutils.cpp (18 lines)
M			plugins/cmake/settings/cmakepreferences.cpp (6 lines)
M			plugins/cmakebuilder/cmakejob.cpp (6 lines)