Review of update tasks:

  • Good job on making good task descriptions; task names should be more concise, though ("update availability class names", for example)
  • If you use slf4j, have a static final logger instance. (change your class template in your ide?), e.g.
    private static final org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(RenameContentConnectorPathPropertyTask.class);
    

    instead of

    private Logger log = LoggerFactory.getLogger(getClass());
    
  • Don't catch/log. Rethrow (if you have a RepositoryException you can't treat, you probably should stop the update altogether). And if you need to log something ("hey, we noticed your config is outdated, blablabla"), then do that via InstallContext#warn()/#error()/#info()


  • In ChangeJcrDependentAvailabilityRuleClassesFqcnTask
    } catch (RepositoryException e) {
      log.error("Unable to process app node ", e);
    
    • rethrow or treat.
    • either way, make sure the message is accurate ("unable to process app node", is neither precise nor correct)
  • Couldn't MigrateRuleClassToAvailabilityRuleDefinitionCollectionTask be merged with the above ? They run the same query - i.e operate on all 'availability' nodes.
  • For tasks aren't meant to be re-used by other modules, mark them as such (i.e by not making them public)
  • If MigrateWorkspaceAndPathToContentConnector is meant to be re-used, make it explicit (javadoc?)
    • I'm on the fence regarding that one being reused or doing the job for other modules. Modules will have to update their bootstraps, so they might as well handle their updates. No strong feeling.
Change By: Grégory Joseph (21/Mar/14 3:39 PM)
Resolution: Fixed
Status: Resolved Reopened
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira



----------------------------------------------------------------
For list details, see: http://www.magnolia-cms.com/community/mailing-lists.html
Alternatively, use our forums: http://forum.magnolia-cms.com/
To unsubscribe, E-mail to: <[email protected]>
----------------------------------------------------------------

Reply via email to