Differential D28986

API dox: document Q_DECLARE_FLAGS-based flags
ClosedPublic
Actions

Authored by kossebau on Apr 19 2020, 4:56 PM.

Details

Reviewers

dfaure

Group Reviewers

Frameworks

Commits

R244:3d30318e4d37: API dox: document Q_DECLARE_FLAGS-based flags

Summary

KApiDox & ECMAddQch have been just teached about Q_DECLARE_FLAGS, so the
underlying typedefs are no longer skipped by doxygen, but can be
documented now, allowing links to be generated for these types e.g. when
used as arguments.

The "#" prefix to the enum name in the description text of all the
Q_DECLARE_FLAGS docs seems needed to properly trigger doxygen autolinks.

Diff Detail

Repository

R244 KCoreAddons

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

kossebau created this revision.Apr 19 2020, 4:56 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptApr 19 2020, 4:56 PM

Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript

kossebau requested review of this revision.Apr 19 2020, 4:56 PM

Harbormaster completed remote builds in B25469: Diff 80562.Apr 19 2020, 4:56 PM

dfaure added inline comments.Apr 19 2020, 5:18 PM

src/lib/jobs/kjob.h
99	Inexperienced developers might be confused by the OR in there. In English it sounds like exclusive or, while the whole point of Q_DECLARE_FLAGS is that multiple flags can be set at the same time. Why not just say "Stores a combination of" ?

kossebau added inline comments.Apr 19 2020, 5:30 PM

src/lib/jobs/kjob.h
99	I had myself inspired by the wording of https://doc.qt.io/qt-5/qflags.html and similar of Qt docs for flags like https://doc.qt.io/qt-5/qwidget.html#RenderFlag-enum, which currently also talk about "The QFlags class provides a type-safe way of storing OR-combinations of enum values" or "The RenderFlags type is a typedef for QFlags<RenderFlag>. It stores an OR combination of RenderFlag values." But I tend to think you have a point, the OR here is not really needed and rather confusing, Will update.

drop the "OR" from "OR combination"

Harbormaster completed remote builds in B25474: Diff 80569.Apr 19 2020, 5:33 PM

If okay, I would walk over the other KF modules and apply the same pattern there as well, once this here is accepted.

dfaure accepted this revision.Apr 19 2020, 5:35 PM

This revision is now accepted and ready to land.Apr 19 2020, 5:35 PM

Sure.

Closed by commit R244:3d30318e4d37: API dox: document Q_DECLARE_FLAGS-based flags (authored by kossebau). · Explain WhyApr 19 2020, 5:38 PM

This revision was automatically updated to reflect the committed changes.

Showing Only Differences

This revision modifies 4 more files that are hidden because they were not modified between selected diffs and they have no inline comments.

Revision Contents
Changeset List

			Path	Packages
M			src/lib/jobs/kjob.h (15 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	80562	7d3e832		Apr 19 2020, 4:56 PM	★	★
Diff 2	80569	7d3e832	drop the "OR" from "OR combination"	Apr 19 2020, 5:33 PM	★	★
Diff 3	80571	7d3e832	R244:3d30318e4d373ffcb9806edf9c3629f26eda3292	Apr 19 2020, 5:38 PM	★	★

Diff 80571

View Options