Herewith I'm sending the notes for Stratos 2 CLI Tool Code Review Crucible Project: http://wso2.org/crucible/cru/WCP098-1<http://www.google.com/url?q=http%3A%2F%2Fwso2.org%2Fcrucible%2Fcru%2FWCP098-1&usd=2&usg=AFQjCNGs8QcdUEG-7j7XKF-pNBDgodtSjA>
CLI Tool source code can be found [1]. jline2 [2] is used to handle console input and Apache Commons CLI [3] is used to handle command line arguments. Review Notes: 1. Need to print the usage for invalid options passed from the terminal. (I have already done this in commands. I missed this for the application) 2. It was questioned why slf4j is used instead of commons log. I explained that I have used slf4j since the CLI tool is a standalone application and it is easier to use logging methods in slf4j 3. Discussed the possibility of using CLI tool with Ant, Scripts. It should be possible since we take authentication details from the environment variables. 4. Purpose of "policies" command is not clear. Should rename appropriately. The Stratos Controller policy.xml file should also be renamed to reflect the meaning, i.e. autoscaling policy.xml 5. Discussed the use of "privateRepo". Possibility of figuring out whether repository is private or public. Currently privateRepo is used to request authentication details. I have discussed some issues in a dev@mail thread with subject "[Stratos2][UI Improvement] GIT related fields should come to one box". 6. CLI Tool should prompt username and password for Git Repository instead of printing an error message. Currently the CLI tool just prints an error saying authentication details are required, if the user has specified the given Git Repository URL as a private repository. 7. Should maintain consistency. We shouldn't use camel case for options. For example, "privateRepo" and "repoURL" in subscribe command. 8. In the help, it is good to give a URL to get detailed information about commands etc. (Can link Wiki docs) 9. Discussed to provide a manual page for CLI tool 10. Discussed the possibility of using "make", "make configure" and "make install" to install the CLI tool. 11. Mention the Log4j log file location in Docs or README 12. Check instanceof the object, before casting directly to an "Option". [1] https://svn.wso2.org/repos/wso2/carbon/platform/branches/4.1.0/components/stratos/artifact-deployment-coordinator/org.wso2.carbon.adc.mgt.cli/2.1.3<http://www.google.com/url?q=https%3A%2F%2Fsvn.wso2.org%2Frepos%2Fwso2%2Fcarbon%2Fplatform%2Fbranches%2F4.1.0%2Fcomponents%2Fstratos%2Fartifact-deployment-coordinator%2Forg.wso2.carbon.adc.mgt.cli%2F2.1.3&usd=2&usg=AFQjCNFbBIjL1joQDPoAAMtAy15jGL_Giw> [2] http://jline.github.io/jline2/<http://www.google.com/url?q=http%3A%2F%2Fjline.github.io%2Fjline2%2F&usd=2&usg=AFQjCNHcJ2mxeFQg9OB74fqtHgothnpyyg> [3] http://commons.apache.org/proper/commons-cli/<http://www.google.com/url?q=http%3A%2F%2Fcommons.apache.org%2Fproper%2Fcommons-cli%2F&usd=2&usg=AFQjCNF9JXf3S_XHPZqfg9Bs-34Q34UzCg> -- Isuru Perera Senior Software Engineer | WSO2, Inc. | http://wso2.com/ Lean . Enterprise . Middleware Twitter: http://twitter.com/chrishantha | LinkedIn: http://lk.linkedin.com/in/chrishantha/
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
