D7652: Better broadcast/neighbor networking

[Albert Vaca Cintora]

albertvaka added a comment.


  Thanks for your time working on this. Just two quick comments (I haven't finished reviewing it yet, will continue tomorrow):
  
  - Translations are managed by the KDE translation teams, so we don't need to worry about them :) This includes removing all the instances of 'on_data_message' on languages other than English like you did: this is something that the translation update script would have done automatically for us.
  
  - It doesn't matter this time, but it is usually a good practice to split unrelated changes in separated review requests. It makes reviewing easier and helps to keep a meaningful git history (so, for example, a change can be reverted without reverting everything else). For example, using `TextUtils.join` in the CustomDevicesActivity has nothing to do with the rest of the patch, and it could be merged right away if it was its own patch.
  
  Last thing: I see you are not subscribed to the KDE Connect mailing list [1]. If you want to keep contributing to KDE Connect, it would be a good idea to subscribe so you can take part in discussions and code reviews :)
  
  [1] https://mail.kde.org/mailman/listinfo/kdeconnect

REPOSITORY
  R225 KDE Connect - Android application

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

To: daniel.z.tg, #kde_connect
Cc: albertvaka, daniel.z.tg, jeanv, tfella, aboudhar, seebauer, bugzy, progwolff, MayeulC, menasshock, ach, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdeconnect/attachments/20170904/808016b8/attachment.html>