12 Dec
2009
12 Dec
'09
2:45 p.m.
Hi Vincent,
On Fri, Dec 11, 2009 at 7:24 PM, Vincent Massol <vincent(a)massol.net> wrote:
Many thanks for the review. I agree with all the issues you have mentioned.
Now, except for 5) I would like to address rest of the issues asap (one by
one, alongside my other work). I think I discussed about 6) on the list
about a month ago (http://www.mail-archive.com/devs@xwiki.org/msg10635.html)
but got no replies. I will need to do a small recap and start a new
discussion on this topic.
Thanks.
- Asiri
> Thanks
> -Vincent
>
_______________________________________________
> devs mailing list
> devs(a)xwiki.org
> http://lists.xwiki.org/mailman/listinfo/devs
Hi Asiri,
This is mostly for you but sending here for others to see and comment
if they want. While working on adding support for wiki macro
visibility I have found the following issues in the existing wiki
macro code:
1) Usage of String for document names instead of DocumentName class.
I've fixed this.
2) Printing logs in all methods instead of sending exceptions on
errors. I've fixed this too.
3) Bad test strategy for testing DefaultWikiMacro. It's not needed to
use RenderingTests with test files. I've fixed this.
4) Missing several tests in DefaultWikiMacro. I have not fixed this.
5) Bad strategy for providing user feedback on macro registration. I
believe we shouldn't implement registration as an event listener and
instead have a Wiki Macro management page with register/unregister
buttons and when clicked the result is printed on the page. This
should probably be done as a replacement of the WikiMacros page in the
wiki macro bridge application. WDYT? I've not fixed this.
6) Programming rights required to access macro params. I have not
fixed this.
7) There was a bug with parameter ordering I have fixed a few weeks
ago but I'm mentionning it here for completeness.
8) Context restoration in DefaultWikiMacro. It's currently not in a
finally block and thus it's possible it's not restored if there are
exceptions raised. I have not fixed this.
9) Missing some comments for complex code portions. For ex in
DefaultWikiMacro for the inline macro stuff. I have not fixed this.
10) Systematic setting of null values not required in lots of place. I
have fixed all I saw.
11) DefaultWikiMacro only executes Macro Transformations but not other
Transformations. I think this is a problem. I have not fixed this.
12) Several classes had full javadoc but the checkstyle config wasn't
set in the POM, thus allowing checkstyle errors to creep in unnoticed.
I have only partially fixed this. Please check all classes that can be
added and fix the few that still have errors.
13) Missing tests for the Event Listener but because of 5) I haven't
added them.
14) Missing at least one functional test to prove everything works.