> On Sept. 12, 2013, 8:17 p.m., Matěj Laitl wrote: > > 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).
Thats the reason why i made it a config option. Do you suggest the default should be plain HTTP? > On Sept. 12, 2013, 8:17 p.m., Matěj Laitl wrote: > > src/context/engines/wikipedia/WikipediaEngine.cpp, line 92 > > <http://git.reviewboard.kde.org/r/112706/diff/1/?file=189221#file189221line92> > > > > Ditto m_ prefix. resolved. > On Sept. 12, 2013, 8:17 p.m., Matěj Laitl wrote: > > src/context/engines/wikipedia/WikipediaEngine.cpp, line 48 > > <http://git.reviewboard.kde.org/r/112706/diff/1/?file=189221#file189221line48> > > > > minor: code style: spaces resolved. > On Sept. 12, 2013, 8:17 p.m., Matěj Laitl wrote: > > src/context/engines/wikipedia/WikipediaEngine.cpp, line 179 > > <http://git.reviewboard.kde.org/r/112706/diff/1/?file=189221#file189221line179> > > > > I believe this debugging is not needed. removed. > On Sept. 12, 2013, 8:17 p.m., Matěj Laitl wrote: > > src/context/applets/wikipedia/WikipediaApplet_p.h, line 146 > > <http://git.reviewboard.kde.org/r/112706/diff/1/?file=189219#file189219line146> > > > > 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. Resolved. ACK about d-pointers. - Frank ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112706/#review39920 ----------------------------------------------------------- On Sept. 12, 2013, 7: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, 7: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
