Hi Sachith, Good work . I have merged your pull request [A] since pack need to be tested with this. Please do following changes to your next immediate pull request. Also schedule a code review for Monday.
[A]. https://github.com/wso2/carbon-appmgt/pull/24 1. Update licence headers in all files 2. Add class level and method level comments and mentioned what each class/method is doing 3. Exception hanldling in all classes -Remove all "e.printStackTrace()" -Introduce new exception class like "AppMSampleDeployerException" and log the original exception and throw this exception. eg: catch (IOException e) { e.printStackTrace(); } chnage it as, catch (IOException e) { log.error("Error while building login html page", e); throw new AppMSampleDeployerException("Error while building login html page", e); } 4. ApplicationController.java -> - Read ip address from carbon server properties instead of NetworkUtils.getLocalHostname() - Remove hardcoded 8280 port in accsesWebPages() method 5. HttpHandler.java -> - Is this correct "private final static String USER_AGENT = "Mozilla/5.0";" - Also this won't run if the default keystores get changed. Need to discuss whether we need to handle this HttpsURLConnection.setDefaultHostnameVerifier(new HostnameVerifier() { public boolean verify(String hostname, SSLSession session) { if (hostname.equals("localhost")) return true; return false; } }); Regards, Dinusha. -- Dinusha Dilrukshi Senior Software Engineer WSO2 Inc.: http://wso2.com/ Mobile: +94725255071 Blog: http://dinushasblog.blogspot.com/
_______________________________________________ Dev mailing list Dev@wso2.org http://wso2.org/cgi-bin/mailman/listinfo/dev
