add documentation to result class
ClosedPublic

Authored by astippich on Dec 2 2018, 11:18 AM.

Details

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
astippich created this revision.Dec 2 2018, 11:18 AM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptDec 2 2018, 11:18 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
astippich requested review of this revision.Dec 2 2018, 11:18 AM

I had the impression that I have to read a lot of code just to know which variable does what, so I wrote some simple documentation.
Please check if I got anything wrong.

yurchor added inline comments.
astippich updated this revision to Diff 46687.Dec 2 2018, 11:34 AM
  • use param
astippich marked 4 inline comments as done.Dec 2 2018, 11:36 AM

Thanks!

bruns added a comment.Dec 2 2018, 1:37 PM

I think methods reimplemented from and documented in KFileMetaData::ExtractionResult should not be documented here.

src/file/extractor/result.h
35

Remove the last part of the sentence - ... is saved to.. Everything else is an implementation detail.

37

`The results can be retrieved as a Baloo::Document using \c document() and
stored in the database.`

48

This is no overload, but an override/reimplementation.

51

I think a \sa KFileMetaData::Property is sufficient here. Also, its not a property name, that would be something like a QString.

astippich updated this revision to Diff 46706.Dec 2 2018, 2:12 PM
  • implement feedback
astippich marked 4 inline comments as done.Dec 2 2018, 2:13 PM
bruns added inline comments.Dec 2 2018, 3:42 PM
src/file/extractor/result.h
57

The use case is missing here.
Can be used to add extraction results to an existing Baloo::Document. Has to be called before passing the Result to KFileMetaData::Extractor::extract()

61

Thats not correct, see implementation of Result::add(...)

QString p = QString::number(propnum)

But as m_map/map() is never used outside Result (only in Result::finish), it is best to remove the getter completely. D17312

bruns requested changes to this revision.Dec 2 2018, 3:46 PM
bruns added inline comments.
src/file/extractor/result.h
87

Only properties

This revision now requires changes to proceed.Dec 2 2018, 3:46 PM
astippich updated this revision to Diff 46750.Dec 2 2018, 8:34 PM
  • more adjustments
astippich marked 3 inline comments as done.Dec 2 2018, 8:35 PM
bruns added inline comments.Dec 3 2018, 11:37 AM
src/file/extractor/result.h
51

Qt uses "Reimplemented from KFileMetaData::ExtractionResult::addtype()`, see e.g. http://doc.qt.io/qt-5/qfile.html#fileName

Overriden has a typo, "Overridden"

But I think you should remove the (docstring) comment, it should automatically pick up the description from the interface class.

79

Nitpick - missing full stop, also below.

astippich updated this revision to Diff 46864.Dec 4 2018, 7:31 PM
  • remove docstring for reimplemented functions

Thanks for your patience :)

bruns added a comment.Dec 4 2018, 11:46 PM

Thanks for your patience :)

Thanks for bearing with my nagging ;-)

bruns added inline comments.Dec 5 2018, 2:39 PM
src/file/extractor/result.h
79

The TermGenerator's do not contain any data themselves, but

  • keep/update the position state when adding data
  • add the data to the referenced Baloo::Document when supplied with new data (index*Text())
astippich updated this revision to Diff 46987.Dec 6 2018, 6:22 PM
  • rephase description of TermGenerator
astippich marked 2 inline comments as done.Dec 6 2018, 6:23 PM
astippich added inline comments.
src/file/extractor/result.h
79

Would be nice if you could add that information to the TermGenerator :)

bruns accepted this revision.Dec 6 2018, 8:20 PM
This revision is now accepted and ready to land.Dec 6 2018, 8:20 PM
This revision was automatically updated to reflect the committed changes.
astippich marked an inline comment as done.