[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 119991: Use Juvia to add comments on the API class pages
From: "Ben Cooksley" <bcooksley () kde ! org>
Date: 2014-09-12 3:19:13
Message-ID: 20140912031913.8756.62229 () probe ! kde ! org
[Download RAW message or body]
--===============6237198137713276151==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On Sept. 8, 2014, 10:22 p.m., Ben Cooksley wrote:
> > What is the status of this? Is there anything blocking it being shipped and made \
> > available on api.kde.org?
>
> Denis Steckelmacher wrote:
> It works locally, so I'm just waiting for a ship-it or other comments :-)
>
> Aleix Pol Gonzalez wrote:
> Let's see if we can get this discussed in the KF5 BoF today.
>
> Aleix Pol Gonzalez wrote:
> I kind of forgot... :(
>
> Anyway, +1 for me. If there are no reasons to keep postponing it, I'd say let's \
> just do this.
> Matthew Dawson wrote:
> The only issue I see is that the email notifications has the admin url included, \
> which it appears I cannot access, making it hard to find the new comment. There \
> was a suggested change to juvia, which would fix this. Has that been applied?
> Otherwise, +1 for me too.
>
> Ben Cooksley wrote:
> I have not seen a reference to that. If it requires changes to Juvia itself, please \
> submit them upstream then file a sysadmin ticket so we can integrate them. I don't \
> see why that would prevent us from moving forward with deploying this change as it \
> can be fixed later without changing the api.kde.org side code.
> Based on the feedback above, I think this can now be committed Denis.
>
> Matthew Dawson wrote:
> I don't think upstream would be happy with the patch, as it hijacks the moderator \
> emails (which should link to the admin page) to send out notifications of when a \
> comment occurs. There was a suggestion on how to change the link in the email in a \
> comment above (not a formal patch), which we could apply locally. If syadmins \
> don't want to carry the change locally (which I understand completely), then we \
> should see about submitting such a feature upstream.
> Regardless of whether the change is kept locally, I agree it can be committed. \
> I'll throw the ship it switch, since it seems we are all in agreement.
Hm, indeed. If someone could prepare such a patch that would be much appreciated. \
Perhaps it is necessary for us to add such functionality?
- Ben
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119991/#review66088
-----------------------------------------------------------
On Aug. 29, 2014, 2:16 p.m., Denis Steckelmacher wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119991/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 2:16 p.m.)
>
>
> Review request for KDE Frameworks and Aurélien Gâteau.
>
>
> Repository: kapidox
>
>
> Description
> -------
>
> Juvia is a Free Software commenting system that can easily be used on static \
> websites like api.kde.org. An instance of Juvia has just been installed on \
> commenting.kde.org (many thanks to Ben Cooksley!), and this patch adds support for \
> it to api.kde.org.
> The users can now comment class pages. The comments are disabled (no comment box \
> nor anything else appears) on the main page of each framework, on the Frameworks 5 \
> index page and on any other page that does not directly concern a class. I've done \
> that in order to avoid cluttering important pages with comments, but if you think \
> that having comments on all the pages (or a bigger subset of them) is desirable, it \
> is very easy to change. Personally, I would avoid having comments on the main \
> pages, so that any spam, if the automatic Akismet filter does not work, will not be \
> too visible.
> A screenshot is linked to this review request and shows how the comments have been \
> integrated (I slightly modified the built-in Juvia style so that Doxygen and KDE \
> colors are used).
>
> Diffs
> -----
>
> src/kapidox/data/htmlresource/kde.css e173dfe
> src/kapidox/data/templates/comments.html PRE-CREATION
> src/kapidox/data/templates/doxygen.html d00e14e
>
> Diff: https://git.reviewboard.kde.org/r/119991/diff/
>
>
> Testing
> -------
>
> Posting a comment works, and comments can be viewed. They also appear in the admin \
> interface of Juvia (I've now deleted these comments). I posted comments on \
> different classes and in different frameworks in order to test that namespacing \
> works correctly.
>
> File Attachments
> ----------------
>
> Comments on api.kde.org
> https://git.reviewboard.kde.org/media/uploaded/files/2014/08/29/278299d8-18a6-46a0-ada5-1b6452a3276f__apidox-comments-1.png
>
>
> Thanks,
>
> Denis Steckelmacher
>
>
--===============6237198137713276151==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119991/">https://git.reviewboard.kde.org/r/119991/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On September 8th, 2014, 10:22 p.m. UTC, <b>Ben \
Cooksley</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">What is the status of this? Is there anything blocking \
it being shipped and made available on api.kde.org?</p></pre> </blockquote>
<p>On September 9th, 2014, 7:33 a.m. UTC, <b>Denis Steckelmacher</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It \
works locally, so I'm just waiting for a ship-it or other comments :-)</p></pre> \
</blockquote>
<p>On September 9th, 2014, 9:12 a.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Let's \
see if we can get this discussed in the KF5 BoF today.</p></pre> </blockquote>
<p>On September 10th, 2014, 8:02 a.m. UTC, <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
kind of forgot... :(</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Anyway, +1 for me. If there are no \
reasons to keep postponing it, I'd say let's just do this.</p></pre> </blockquote>
<p>On September 12th, 2014, 2:06 a.m. UTC, <b>Matthew Dawson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The \
only issue I see is that the email notifications has the admin url included, which it \
appears I cannot access, making it hard to find the new comment. There was a \
suggested change to juvia, which would fix this. Has that been applied?</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Otherwise, +1 for me too.</p></pre> </blockquote>
<p>On September 12th, 2014, 2:48 a.m. UTC, <b>Ben Cooksley</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
have not seen a reference to that. If it requires changes to Juvia itself, please \
submit them upstream then file a sysadmin ticket so we can integrate them.<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" /> I don't see why that would prevent us from moving forward with deploying \
this change as it can be fixed later without changing the api.kde.org side code.</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Based on the feedback above, I think this can now be \
committed Denis.</p></pre> </blockquote>
<p>On September 12th, 2014, 3:07 a.m. UTC, <b>Matthew Dawson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
don't think upstream would be happy with the patch, as it hijacks the moderator \
emails (which should link to the admin page) to send out notifications of when a \
comment occurs. There was a suggestion on how to change the link in the email in a \
comment above (not a formal patch), which we could apply locally. If syadmins don't \
want to carry the change locally (which I understand completely), then we should see \
about submitting such a feature upstream.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Regardless of whether \
the change is kept locally, I agree it can be committed. I'll throw the ship it \
switch, since it seems we are all in agreement.</p></pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Hm, \
indeed. If someone could prepare such a patch that would be much appreciated. Perhaps \
it is necessary for us to add such functionality?</p></pre> <br />
<p>- Ben</p>
<br />
<p>On August 29th, 2014, 2:16 p.m. UTC, Denis Steckelmacher wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for KDE Frameworks and Aurélien Gâteau.</div>
<div>By Denis Steckelmacher.</div>
<p style="color: grey;"><i>Updated Aug. 29, 2014, 2:16 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kapidox
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Juvia is a Free Software commenting system that can \
easily be used on static websites like api.kde.org. An instance of Juvia has just \
been installed on commenting.kde.org (many thanks to Ben Cooksley!), and this patch \
adds support for it to api.kde.org.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">The users can now \
comment class pages. The comments are disabled (no comment box nor anything else \
appears) on the main page of each framework, on the Frameworks 5 index page and on \
any other page that does not directly concern a class. I've done that in order to \
avoid cluttering important pages with comments, but if you think that having comments \
on all the pages (or a bigger subset of them) is desirable, it is very easy to \
change. Personally, I would avoid having comments on the main pages, so that any \
spam, if the automatic Akismet filter does not work, will not be too visible.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">A screenshot is linked to this review request and shows how the comments \
have been integrated (I slightly modified the built-in Juvia style so that Doxygen \
and KDE colors are used).</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Posting a comment works, and comments can be viewed. \
They also appear in the admin interface of Juvia (I've now deleted these comments). I \
posted comments on different classes and in different frameworks in order to test \
that namespacing works correctly.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/kapidox/data/htmlresource/kde.css <span style="color: \
grey">(e173dfe)</span></li>
<li>src/kapidox/data/templates/comments.html <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/kapidox/data/templates/doxygen.html <span style="color: \
grey">(d00e14e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119991/diff/" style="margin-left: \
3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments \
</h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/08/29/278299d8-18a6-46a0-ada5-1b6452a3276f__apidox-comments-1.png">Comments \
on api.kde.org</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============6237198137713276151==--
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic