D5941: Code refactoring, commening, and implement enough to have kdeconnect show as an option when adding an account!

Simon Redman noreply at phabricator.kde.org
Wed May 24 01:32:17 UTC 2017


sredman marked an inline comment as done.
sredman added inline comments.

INLINE COMMENTS

> apol wrote in CMakeLists.txt:16
> Ugh no, don't do that. Rename it if you want, but don't pass the name with a variable.
> Better be explicit.

Thank you for the feedback! I have never used CMake before, so I have not seen the failure case using a variable for the outputs would cause. Do you have a horror story (which you would like to share), or is this just general good practice?

> apol wrote in connection.cpp:175
> You added one comment in the cpp and the other on the header. Please do the same on both.

Do you mean I should have, for method-header comments, the same thing in both the .h and .cpp files? The reason I don't want to do this (at least not at present) is that everything is changing quite rapidly. It would be hard to keep the two sets of comments in sync (and they would surely get out of sync)

To that end, I have been keeping all my header comments in the .cpp file. The comment in the .h file for the messageReceived(..) signal is only in the .h file because it doesn't appear in the .cpp

In general, I like using something like doxygen to keep track of comments regardless. This solves the problem of wondering whether a header comment belongs in the source or header file, because it is easier and nicer-looking to refer to the doxygen output in any case. I have added a Doxyfile (and the related output to the .gitignore)

REPOSITORY
  R716 Telepathy KDE Connect Integration

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

To: sredman, kdeconnect, davidedmundson
Cc: apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20170524/6a3be8d9/attachment.html>


More information about the KDEConnect mailing list