[OOTB-addons] Uploader-Plus - Informal Review

[Axel Faust]

Hello,

after getting back from a two-week engagement, I finally got some time to check this.
Thanks Angel for doing this informal review and the feedback.

My notes / questions on this:

The violation of non-technical #7 does not apply (any more) at this point. The addon does not require a build from source (which is the requirement) and provides pre-built AMP on the GitHub release page (https://github.com/softwareloop/uploader-plus/releases/tag/v1.0).

I’ve removed the duplicate technical #19

I’ve added technical #31 as a “should not” for the issue of overriding FTL you listed. But he actually uses the web-extension approach which we check for in technical #13, so technically the addon uses one of the recommended approaches – although it would be way better if he used Surf Sub-Component configuration via the extension module.
I will check if his customizations would be possible that way and either file an issue with him or prepare a pull-request for it.

I agree with the added value proposition, but previous discussions have taught me to separate them from the review aspect. A reviewer should focus on the as-is state of an addon and not be required to do the work of the developer. I feel it should be a priority to give feedback to the developer so he/she can provide the added value, e.g. like Paolo has apparently added the pre-built AMPs and even done his own compatibility tests (http://softwareloop.com/uploader-plus-compatibility-tests/).

Regarding review tooling: We definitely have to look into potential tools that offer high value for limited effort. But we should collect some more input from reviews to find the really boring / tedious parts of a review to try and optimize. What aspect of the code “inspection” do you feel to require the most effort?

Regards
Axel

Von: OOTB-addons [mailto:ootb-addons-bounces at xtreamlab.net] Im Auftrag von Angel Borroy
Gesendet: Sonntag, 16. November 2014 21:09
An: ootb-addons at xtreamlab.net
Betreff: [!!Mass Mail]Re: [OOTB-addons] Documenting / organising acceptance criteria

Uploader Plus - Informal Review

URL: http://softwareloop.com/uploader-plus-an-alfresco-uploader-that-prompts-for-metadata/
Github: https://github.com/softwareloop/uploader-plus

Non-technical
1, 2, 3 - License GNU LGPL 3
4 - Only Alfresco libraries
5 - Non informed
6 - Maven
7 - Not currently on addons (but inboxes from the same author is in)
8 - http://softwareloop.com/uploader-plus-installation/ (outside addons)
9 - https://github.com/softwareloop/uploader-plus
10 - http://softwareloop.com/uploader-plus-installation/, http://softwareloop.com/uploader-plus-configuration/, http://softwareloop.com/uploader-plus-working-with-custom-content-types/
11 - Only Alfresco libraries are required (not listed)

Technical
1 - AMP
2 - AMP
3 - AMP
4 - /
5 - /
6 - AMP
7 - NO (We can provide AMP ready to deploy, it should be our add value on this)
8 - share + repo
9 - ok
10 - AMP
11 - AMP + Maven SDK
12 - Maven
13 - ok
14 - ok
15 - ok
16 - ok
17 - ok
18 - ok
19 - ok (REPEAT 18)
20 - ok
21 - ok
22 - ok
23 - ok
24 - ok
25 - ok
26 - ok
27 - ok
28 - ok
29 - ok
30 - ok

It overrides dnd-upload.get.html.ftl, flash-upload.get.html.ftl, html-upload.get.html.ftl but we have no check on this.

Compatibility
1 - 4.2.f
2 - ok
3 - ok
4 - Chrome / Firefox / Safari (tested by me, it could be another add value from OOTB)
5 - ok
6 - ok

Conclusions

  *   This addon should be in if there were AMP ready to deploy and if FTL override should be accepted
  *   Tech 18 and Tech 19 are repeated in our check list
  *   OOTB add value possibilities

     *   Provide ready to deploy AMPs
     *   Test addons on common browsers

  *   We need to include FTL extension criteria on our Tech section?
  *   My review has been done examining the code by my own, but it should be nice to have some tools in order to inspect Alfresco bean utilization, use of Alfresco API and any other check that should be checked programmatically
On the other hand, I’ve written to Paolo to thank him this beauty code and to suggest him one possible addition. Should we do this beside OOTB brand?

Finally, on the subjective point of view. I think it’s a great code to adapt to many projects but I feel it’s not a ready to use artifact. However, there are few modifications to be done to get this ready to use level.

Regards,

<https://github.com/softwareloop/uploader-plus>

Angel Borroy
keensoft

email:   angel.borroy at keensoft.es<mailto:angel.borroy at keensoft.es>
web:     http://www.keensoft.es<http://www.keensoft.es/>
móvil:  +34 655 47 47 55

CONFIDENCIALIDAD:

La presente comunicación y, en su caso, los ficheros que lleve adjuntos, pertenecen exclusivamente a las personas a las que va dirigido y puede contener información confidencial. Si usted no es el destinatario de este mensaje (o la persona responsable de su entrega), considérese advertido de que lo ha recibido por error y que cualquier uso, difusión, reenvío o copia están prohibidos legalmente. Si ha recibido este mensaje por error, por favor notifíquelo al remitente y proceda a destruirlo inmediatamente.

This message and the attached documents may contain privileged/confidential information and intended solely for the use of the individual to whom it is addressed. If you are not the intended recipient (or responsible for delivery of the message to such a person) be advised that you have received this message in error and that any use, dissemination, forwarding, printing or copying of this e-mail is strictly prohibited. If you have received this message in error please notify it to the sender and destroy it immediately.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.xtreamlab.net/pipermail/ootb-addons/attachments/20141123/be151960/attachment-0001.html>