[OOTB-addons] Uploader-Plus - Informal Review

[Angel Borroy]

Hi, 

To inspect addons like uploader plus, with just only two dozens of files, is a reasonable effort and it can be done by hand. But what about more complex addons? Should we invest some time on experimenting with SonarQube rules or something similar? I have no experience on this, but it seems a suitable approach.

On the other hand, our reviews should be a public work and we should engage developers on it in order to improve developments quality and some homogenization. So, we should explore some tool able to provide a formatted review ready to publish in some way (github wiki could be enough?)

Finally, Xmas is here and we have no work to show. Should we get a candidate addons list and get some of them reviewed? I can provide one or two formal reviews in the next two/three weeks, but we need to fix our target addons before this.

Regards,

Angel Borroy
keensoft

email:   angel.borroy at keensoft.es
web:     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.


En 23 de noviembre de 2014 en 20:49:06, Axel Faust (axel.faust at prodyna.com) escrito:

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,



Angel Borroy
keensoft

email:   angel.borroy at keensoft.es
web:     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/20141128/74e00d43/attachment-0001.html>