Hi Dinusha, OK.I will add those changes.
Regards, *Sachith Ushan* Software Engineering intern WSO2 : http://wso2 <http://goog_1352065705>.com mobile :- +94 71 8853336 sachi...@wso2.com On Fri, Apr 3, 2015 at 1:17 PM, Dinusha Senanayaka <dinu...@wso2.com> wrote: > 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
