Review Request: Draft of a new AddAccountUi
David Edmundson
kde at davidedmundson.co.uk
Wed Dec 28 14:22:39 UTC 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103562/#review9329
-----------------------------------------------------------
Much nicer.
src/KCMTelepathyAccounts/simple-profile-select-widget.h
<http://git.reviewboard.kde.org/r/103562/#comment7708>
Still not you.
src/KCMTelepathyAccounts/simple-profile-select-widget.cpp
<http://git.reviewboard.kde.org/r/103562/#comment7709>
never use "magic numbers"- having random numbers about in your code is generally bad.
Either:
1) create an enum and use that
2) use strings in the mapper (you can pass the service name here and then you don't need that switch statement)
src/add-account-assistant.cpp
<http://git.reviewboard.kde.org/r/103562/#comment7706>
else {
}
src/add-account-assistant.cpp
<http://git.reviewboard.kde.org/r/103562/#comment7707>
braces {}
- David Edmundson
On Dec. 28, 2011, 2:13 p.m., Florian Reinhard wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103562/
> -----------------------------------------------------------
>
> (Updated Dec. 28, 2011, 2:13 p.m.)
>
>
> Review request for Telepathy.
>
>
> Description
> -------
>
> This patch is not intended for master. Consider it to be a more detailed mockup, I know the code is crappy ;)
>
> Draw backs:
> * crashes from time to time because of ugly hacks in ProfileItem / SimpleProfileSelectWidget::selectedProfile()
> * profile names are hardcoded, I guess this shouldn't stay this way?
> * profiles from the first page are not excluded from the second page
> * the naming of page one, two, three is hard to understand when reading the code
> * if you click on "Other" you get was formerly was the entry page, one could use the new shiny buttons there too.
>
>
> This addresses bug 279046.
> http://bugs.kde.org/show_bug.cgi?id=279046
>
>
> Diffs
> -----
>
> src/KCMTelepathyAccounts/CMakeLists.txt 0991b4dd3a65d34746e3c92d09c43c4cbced8e92
> src/KCMTelepathyAccounts/profile-item.h dc492a5e37192d91833348e1e1dfe9c96fe41f51
> src/KCMTelepathyAccounts/profile-item.cpp de42b521cbfb047dabf1c5c85decc47dceaac54e
> src/KCMTelepathyAccounts/profile-select-widget.h 52c6f898728dab5ddd8c73548caeea99ac271efe
> src/KCMTelepathyAccounts/profile-select-widget.cpp dc8b9cc1d0bad9c856834c4e0dbae7b9102dee89
> src/KCMTelepathyAccounts/simple-profile-select-widget.h PRE-CREATION
> src/KCMTelepathyAccounts/simple-profile-select-widget.cpp PRE-CREATION
> src/KCMTelepathyAccounts/simple-profile-select-widget.ui PRE-CREATION
> src/add-account-assistant.h 9be243ffbe2d246042ed3684f696f0fccbc1252e
> src/add-account-assistant.cpp dee6e88c51c72f36a1e3099a272db71443a7194f
>
> Diff: http://git.reviewboard.kde.org/r/103562/diff/diff
>
>
> Testing
> -------
>
>
> Screenshots
> -----------
>
>
> http://git.reviewboard.kde.org/r/103562/s/384/
>
>
> Thanks,
>
> Florian Reinhard
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20111228/27b852c0/attachment-0001.html>
More information about the KDE-Telepathy
mailing list