Differential D13706 Diff 36617 src/kmoretools/kmoretools.h

Changeset View

Standalone View

View Options

src/kmoretools/kmoretools.h

Show First 20 Lines • Show All 479 Lines • ▼ Show 20 Line(s)380public:
480 /** 480​ /**
481 * Will override the "Exec=" line of the service. Will only apply if the 481​ * Will override the "Exec=" line of the service. Will only apply if the
482 * service is installed. 482​ * service is installed.
483 * 483​ *
484 * @see KService::setExec(...) 484​ * @see KService::setExec(...)
485 */ 485​ */
486 void setExec(const QString& exec); 486​ void setExec(const QString& exec);
487 487​
488​ /**
dhaumannUnsubmitted
Done

API documentation is missing. Please also add @since 5.48, since this will be the next frameworks release, see: https://community.kde.org/Schedules/Frameworks

dhaumann: API documentation is missing. Please also add @since 5.48, since this will be the next…
489​ * @since 5.48
490​ * @return The service' appstream:// url if available
dhaumannUnsubmitted
Done

What about this:

/**
 * Returns the associated appstream id that was previously set with setAppstreamId().
 * If no appstream id was set, an empty string is returned.
 *
 * @return The service's appstream id.
 *
 * @since 5.48
 */
dhaumann: What about this: /** * Returns the associated appstream id that was previously set…
491​ */
gregormiUnsubmitted
Done

Please add a comment and add something like "@since 5.xx" (see elsewhere in the this file) to indicate since which frameworks version this will be available.

gregormi: Please add a comment and add something like "@since 5.xx" (see elsewhere in the this file) to…
492​ QUrl appstreamUrl() const;
493​
494​ /**
495​ * @since 5.48
dhaumannUnsubmitted
Done

Please move @since down. The tags are usually the last part in the API documentation.

dhaumann: Please move @since down. The tags are usually the last part in the API documentation.
496​ * Sets the appstream:// url that can be used to install the service using a
497​ * software center application (e.g. Discover).
498​ * E.g. appstream://org.kde.spectacle.desktop
499​ */
500​ void setAppstreamUrl(const QUrl& url);
gregormiUnsubmitted
Done

I am not so familiar with appstream. Why not only setting the COMPONENT-ID instead of the whole URL? Then the method would be named setAppstreamComponentId.

gregormi: I am not so familiar with appstream. Why not only setting the COMPONENT-ID instead of the whole…
dhaumannUnsubmitted
Done

This is also missing the parameter, which can be used for documentation:

/**
 * Sets the appstream id of the service. This is used to create a
 * appstream url for installing the service via a software store
 * (e.g. Discover). For instance, the appstream id for filelight is
 * "org.kde.filelight.desktop".
 *
 * @param id the appstream id
 *
 * @since 5.48
 */
void setAppstreamId(const QString& id);
dhaumann: This is also missing the parameter, which can be used for documentation: /** * Sets…
501​
488private: 502​private:
489 /** 503​ /**
490 * @param kmtDesktopfileSubdir where to find kmt-desktopfiles 504​ * @param kmtDesktopfileSubdir where to find kmt-desktopfiles
491 * @param desktopEntryName name of the desktopfile without the .desktop extension 505​ * @param desktopEntryName name of the desktopfile without the .desktop extension
492 * @param isInstalled true if desktop file is installed 506​ * @param isInstalled true if desktop file is installed
493 * @param installedService not nullptr if @p isInstalled is true 507​ * @param installedService not nullptr if @p isInstalled is true
494 * @param kmtDesktopfile not null if app-local kmt-desktopfile is found and valid 508​ * @param kmtDesktopfile not null if app-local kmt-desktopfile is found and valid
495 */ 509​ */
▲ Show 20 Lines • Show All 270 Lines • Show Last 20 Lines