Changeset View
Changeset View
Standalone View
Standalone View
src/core/slavebase.h
Show First 20 Lines • Show All 342 Lines • ▼ Show 20 Line(s) | 63 | public: | |||
---|---|---|---|---|---|
343 | * @since 5.64 | 343 | * @since 5.64 | ||
344 | */ | 344 | */ | ||
345 | QMap<QString, QVariant> mapConfig() const; | 345 | QMap<QString, QVariant> mapConfig() const; | ||
346 | 346 | | |||
347 | /** | 347 | /** | ||
348 | * Returns a bool from the config/meta-data information. | 348 | * Returns a bool from the config/meta-data information. | ||
349 | * @since 5.64 | 349 | * @since 5.64 | ||
350 | */ | 350 | */ | ||
351 | bool configValue(QString key, bool defaultValue) const; | 351 | bool configValue(const QString &key, bool defaultValue) const; | ||
kossebau: Would be a proper change, but sadly also changes the signature of a API under ABI promise, so… | |||||
Ah, this is @since 5.64, so not yet published, so still can be changed. kossebau: Ah, this is @since 5.64, so not yet published, so still can be changed.
Sorry, my comment was… | |||||
352 | 352 | | |||
353 | /** | 353 | /** | ||
354 | * Returns an int from the config/meta-data information. | 354 | * Returns an int from the config/meta-data information. | ||
355 | * @since 5.64 | 355 | * @since 5.64 | ||
356 | */ | 356 | */ | ||
357 | int configValue(QString key, int defaultValue) const; | 357 | int configValue(const QString &key, int defaultValue) const; | ||
358 | 358 | | |||
359 | /** | 359 | /** | ||
360 | * Returns a QString from the config/meta-data information. | 360 | * Returns a QString from the config/meta-data information. | ||
361 | * @since 5.64 | 361 | * @since 5.64 | ||
362 | */ | 362 | */ | ||
363 | QString configValue(QString key, const QString &defaultValue=QString()) const; | 363 | QString configValue(const QString &key, const QString &defaultValue = QString()) const; | ||
while touching the line, please also add spaces around the = -> defaultValue = QString()) kossebau: while touching the line, please also add spaces around the = -> `defaultValue = QString())` | |||||
364 | 364 | | |||
365 | #if KIOCORE_ENABLE_DEPRECATED_SINCE(5, 64) | 365 | #if KIOCORE_ENABLE_DEPRECATED_SINCE(5, 64) | ||
366 | /** | 366 | /** | ||
367 | * Returns a configuration object to query config/meta-data information | 367 | * Returns a configuration object to query config/meta-data information | ||
368 | * from. | 368 | * from. | ||
369 | * | 369 | * | ||
370 | * The application provides the slave with all configuration information | 370 | * The application provides the slave with all configuration information | ||
371 | * relevant for the current protocol and host. | 371 | * relevant for the current protocol and host. | ||
▲ Show 20 Lines • Show All 678 Lines • Show Last 20 Lines |
Would be a proper change, but sadly also changes the signature of a API under ABI promise, so here and in all other public API places not possibe.
Can be only done when breaking the ABI with KF6.
Should there be something else done instead? IMHO no, neither adding a TODO or even already an overload of the method with the preferred signature.
For one, checking and improving all API of KF6 with clazy should be expected to be done before any first release, so this should be catched at that time. At the same time, this change is source-compatible, client-side code does not have to be changed, so there is no real gain in client code readability, and the runtime price tag of a flat QString copy here is equally small compared to having another symbol and implementation code, so less code is better here.
See also https://community.kde.org/Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts