Closed douglascrp closed 8 years ago
Hi Douglas,
Let me start with Nice review! I just have one small remark.
In your Suggestion you've stated the following: Minor re-organisation of configuration
Can you explain this a bit more? Probably it's crystel clear for you, but I'd recommend something like:
The issues identified concerning no 22 and 23 (runAs) should also be explicit parts of the suggested tasks. In this review there are issues with 4 "must" / "must not" criteria, albeit only comparatively harmless ones. We may want to include either a separate column for "pass", "pass (barely)", "fail", or include a short statement in "Notes" to explain/differentiate a non-critical issue better.
@shazada The reorganisation (based on the criteria) only addresses the location in alfresco/extension/templates/webscripts vs. alfresco/templates/webscripts
Ideally, no runAs should not be needed at all as the ability to change the logo is / should be limited to the SiteManager who already has the necessary privileges. So the web script may not need to be refactored into a Java one, and the only change may be to drop the runAs from the .desc.xml
As discussed with @douglascrp via IRC I have merged the review as-is. Douglas will check the feedback and make updates at a later time (likely new PR).
I've just finished my first addon validation. Please, let me know what you think and if I should be doing something else.