D17301: add documentation to result class

2018-12-11 Thread Alexander Stippich 
This revision was automatically updated to reflect the committed changes.
astippich marked an inline comment as done.
Closed by commit R293:60d6fc82ae5a: add documentation to result class (authored 
by astippich).

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17301?vs=46987=47330

REVISION DETAIL
  https://phabricator.kde.org/D17301

AFFECTED FILES
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-06 Thread Stefan Brüns 
bruns accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  result_documentation

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-06 Thread Alexander Stippich 
astippich marked 2 inline comments as done.
astippich added inline comments.

INLINE COMMENTS

> bruns wrote in 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()`)

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

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-06 Thread Alexander Stippich 
astippich updated this revision to Diff 46987.
astippich added a comment.


  - rephase description of TermGenerator

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17301?vs=46864=46987

BRANCH
  result_documentation

REVISION DETAIL
  https://phabricator.kde.org/D17301

AFFECTED FILES
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-05 Thread Stefan Brüns 
bruns added inline comments.

INLINE COMMENTS

> result.h:79
> +/**
> + * Contains the collection of words of the metadata.
> + */

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()`)

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-04 Thread Stefan Brüns 
bruns added a comment.


  In D17301#371323 , @astippich 
wrote:
  
  > Thanks for your patience :)
  
  
  Thanks for bearing with my nagging ;-)

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-04 Thread Alexander Stippich 
astippich added a comment.


  Thanks for your patience :)

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-04 Thread Alexander Stippich 
astippich updated this revision to Diff 46864.
astippich added a comment.


  - remove docstring for reimplemented functions

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17301?vs=46750=46864

BRANCH
  result_documentation

REVISION DETAIL
  https://phabricator.kde.org/D17301

AFFECTED FILES
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-03 Thread Stefan Brüns 
bruns added inline comments.

INLINE COMMENTS

> result.h:58
> +/**
> + * Overriden function from KFileMetaData::ExtractionResult.
> + */

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.

> result.h:90
> +/**
> + * Contains the collection of words of the metadata
> + */

Nitpick - missing full stop, also below.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich marked 3 inline comments as done.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich updated this revision to Diff 46750.
astippich added a comment.


  - more adjustments

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17301?vs=46706=46750

BRANCH
  result_documentation

REVISION DETAIL
  https://phabricator.kde.org/D17301

AFFECTED FILES
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Stefan Brüns 
bruns requested changes to this revision.
bruns added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> result.h:105
> +/**
> + * Contains all indexed data from the extractors
> + */

Only properties

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Stefan Brüns 
bruns added a dependency: D17312: Remove unused map() getter from Result.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Stefan Brüns 
bruns added inline comments.

INLINE COMMENTS

> result.h:83
> + */
>  void setDocument(const Baloo::Document& doc);
>  

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()`

> result.h:87
> + * Returns a QVariantMap containing all metadata fetched from the 
> extractors.
> + * The metadata is saved as 
> + */

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 


REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich marked 4 inline comments as done.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich updated this revision to Diff 46706.
astippich added a comment.


  - implement feedback

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17301?vs=46687=46706

BRANCH
  result_documentation

REVISION DETAIL
  https://phabricator.kde.org/D17301

AFFECTED FILES
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Stefan Brüns 
bruns added a comment.


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

INLINE COMMENTS

> result.h:38
> + * \brief The result class is where all the data extracted by
> + * the KFileMetaData extractors is saved to a Baloo::Document.
> + * It implements the KFileMetaData::ExtractionResult interface.

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

> result.h:40
> + * It implements the KFileMetaData::ExtractionResult interface.
> + * The resulting Baloo::Document can then be pushed into the
> + * database.

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

> result.h:51
> +/**
> + * Overloaded function from KFileMetaData::ExtractionResult.
> + * Adds a key value pair which should be indexed.

This is no overload, but an override/reimplementation.

> result.h:54
> + *
> + * \param property This specifies a property name. It should be one of 
> the
> + * properties from the list of properties from KFileMetaData.

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

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich marked 4 inline comments as done.
astippich added a comment.


  Thanks!

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich updated this revision to Diff 46687.
astippich added a comment.


  - use param

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17301?vs=46684=46687

BRANCH
  result_documentation

REVISION DETAIL
  https://phabricator.kde.org/D17301

AFFECTED FILES
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Yuri Chornoivan 
yurchor added inline comments.

INLINE COMMENTS

> result.h:56
> + *
> + * \p property This specifies a property name. It should be one of the
> + * properties from the list of properties from KFileMetaData.

This should be `\param`

https://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdp

https://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmdparam

> result.h:59
> + *
> + * \p value The value of the property
> + */

Same here.

> result.h:67
> + *
> + * \p text The text that will be appended to the document
> + */

Same here.

> result.h:80
> + * will be saved.
> + * \p doc The Baloo::Document
> + */

Same here.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: yurchor, kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich added a comment.


  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.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich added a dependency: D17300: add result to baloo namespace.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D17301

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams

D17301: add documentation to result class

2018-12-02 Thread Alexander Stippich 
astippich created this revision.
astippich added reviewers: Baloo, bruns.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
astippich requested review of this revision.

REPOSITORY
  R293 Baloo

BRANCH
  result_documentation

REVISION DETAIL
  https://phabricator.kde.org/D17301

AFFECTED FILES
  src/file/extractor/result.h

To: astippich, #baloo, bruns
Cc: kde-frameworks-devel, ashaposhnikov, michaelh, astippich, spoorun, ngraham, 
bruns, abrahams