> On Aug. 31, 2014, 12:57 a.m., Matthew Dawson wrote: > > Thanks for taking a look at this. It appears KConfigBase isn't available > > on api.kde.org as it isn't documented, as kapidox hides such classes by > > default. As KConfigBase is used outside of KConfig, I'd prefer if > > KConfigBase gained a quick blurb to copying its documentation. I think > > something describing it as a way for a function to accept either a KConfig > > or a KConfigGroup and be able load/persist its settings would be good. > > Martin Klapetek wrote: > KConfigBase is very much documented, see [1]. I don't know why it's not > showing up...nevertheless, if that method is reimplementation of the base > class method, it should imho has its own documentation altogether (it's > *reimplementation* after all). What I did here is just a quick fix to improve > that... > > [1] - > http://quickgit.kde.org/?p=kconfig.git&a=blob&h=3d00d98a1eb565df1a6eea2eba165bfeda17978b&hb=5941a608038d799244ba2dfdceb2da8bf1685fc1&f=src%2Fcore%2Fkconfigbase.h > > Matthew Dawson wrote: > While class members of KConfigBase are documented, Doxygen doesn't > consider it documented as the class itself does not have a description > attached. Attaching even a brief line convinces Doxygen otherwise. > > Regarding each method having its own documentation, I took a quick look > through the other code I have checked out on my machine, and it appears most > classes don't copy the documentation from their parent. I couldn't find any > policy on this either from KDE's website. If we want each function in > frameworks to be fully documented, without having to click a reference, then > I'm fine with this patch. This should then probably get documnted on the > community wiki, so others know for the future to include it. > > However, if this patch came from the fact KConfigBase is not on > api.kde.org, then I rather fix this by throwing a quick description on > KConfigBase. Then the sync method's documentation does show up, and a link > is generated on KConfig's sync method making it easy to read. Maybe > something like: > > \brief Interface to interact with configuration. > > KConfigBase allow a component of an application to persist its > configuration, without having to care if it persists its data into a top > level KConfig, or a KConfigGroup inside a KConfig. > > Martin Klapetek wrote: > > it appears most classes don't copy the documentation from their parent. > > Right; my point is that a reimplementation of an abstract class > method/interface method is usually a specific implementation of the broad > parent abstract class meaning. > > > If we want each function in frameworks to be fully documented... if > this patch came from the fact KConfigBase is not on api.kde.org > > In the light of what I wrote above, my intention was a little bit of both. > > Aleix Pol Gonzalez wrote: > Well, if @reimp is passed, it would mean that the parent documentation is > good enough, no? > > Martin Klapetek wrote: > I thought that "@reimp" in doxygen marks the method as reimplemented from > the parent class, ie. it adds a link to it. No? > > Matthew Dawson wrote: > I agree that if the specific implementation have further notes that the > parent's documentation does not address, it would be good to document it. > However, the current patch only copies the documentation from KConfigBase. > KConfigBase isn't on api.kde.org it is not considered documented by Doxygen > (regardless of what Doxygen tags are attached now). Attaching a quick couple > lines to KConfigBase would make it visible, avoiding the copied comment. If > my suggestion above is fine, then I'll commit it. If there is further > documentation that should be attached to KConfig::sync, this RR can be > updated appropriately. > > I do think it would be good to attach some extra documentation to > KConfigGroup::sync, mentioning it syncs the entire file the group is from, as > that might surprise some people. That can be included either in this RR, or > I can add something with KConfigBase's documentation commit. > > Martin Klapetek wrote: > > However, the current patch only copies the documentation from > KConfigBase > > Yes, I wanted to do a "quick fix" and as I don't know much about the > internals, I just copied the base class' docs, which seems fitting enough. > > Ultimately, you're the maintainer, so it's up to you if you add docs to > KConfigBase or extend the sync() comment properly. I mostly wanted to point > out the missing docs and have it fixed quickly. I do agree that it's not the > bestest proper fix ever and I'm happy to discard it for more favorable > solution :)
Alright, I've pushed a commit (http://commits.kde.org/kconfig/fe300454292c519ae5aa4edfcb1b94f3636e59bb) so KConfigBase shows up (and a link now exists between KConfig::sync and KConfigBase::sync). KConfigGroup::sync already had a note, so I didn't change it for now. If you feel it can be helped (the KConfig::sync link isn't obvious, unfortunately), please update this RR. Otherwise please discard this one. - Matthew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119997/#review65567 ----------------------------------------------------------- On Aug. 29, 2014, 5:42 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119997/ > ----------------------------------------------------------- > > (Updated Aug. 29, 2014, 5:42 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kconfig > > > Description > ------- > > The docs at api.kde.org shows just "Reimplemented from superclass." for the > sync() method, however the superclass, KConfigBase, is not available at the > api.kde.org. So this copies the documentation from KConfigBase to KConfig so > the text can actually be visible at api.kde.org > > > Diffs > ----- > > src/core/kconfig.h d7d4b7d > > Diff: https://git.reviewboard.kde.org/r/119997/diff/ > > > Testing > ------- > > > Thanks, > > Martin Klapetek > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
