----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112706/#review39920 -----------------------------------------------------------
Looks good to me (with a few style comments). Please don't forget to add BUG, FIXED-IN tags and ChangeLog entry when comitting. The only reason why we should not hard-code https is that it is blocked by in some circumstances (most prominently by China and Iran goverments). src/context/applets/wikipedia/WikipediaApplet_p.h <http://git.reviewboard.kde.org/r/112706/#comment29458> I believe we don't tend use the m_* prefix for members of Private d-pointer classes. Or perhaps we're not consistent, but please remove it so that it is consistent with surrounding code. The real question is why the d-pointer pattern is applied here at all (and other places in Amarok) - we're not a library a a few positives (apart from not-needed BIC) are IMO greatly outweighted by the added complexity. src/context/engines/wikipedia/WikipediaEngine.cpp <http://git.reviewboard.kde.org/r/112706/#comment29459> minor: code style: spaces src/context/engines/wikipedia/WikipediaEngine.cpp <http://git.reviewboard.kde.org/r/112706/#comment29461> Ditto m_ prefix. src/context/engines/wikipedia/WikipediaEngine.cpp <http://git.reviewboard.kde.org/r/112706/#comment29460> I believe this debugging is not needed. - Matěj Laitl On Sept. 12, 2013, 9:54 p.m., Frank Meerkoetter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112706/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2013, 9:54 p.m.) > > > Review request for Amarok. > > > Description > ------- > > Allow to use wikipedia over SSL. Make encrypted connections the default. Add > an option to fall back to plain HTTP. > > > This addresses bug 322249. > https://bugs.kde.org/show_bug.cgi?id=322249 > > > Diffs > ----- > > src/context/applets/wikipedia/WikipediaApplet.cpp 507db96 > src/context/applets/wikipedia/WikipediaApplet_p.h c52a0bf > src/context/applets/wikipedia/wikipediaGeneralSettings.ui a615dee > src/context/engines/wikipedia/WikipediaEngine.cpp f22e443 > > Diff: http://git.reviewboard.kde.org/r/112706/diff/ > > > Testing > ------- > > Switched several times between HTTP and HTTPS. Switched between the normal > and the mobile version. > Checked the traffic with wireshark. Verified that this setting is actually > persisted. > > > Thanks, > > Frank Meerkoetter > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
