Hi Dinusha,
I sent you a pull request[A] with above changes.
A. https://github.com/wso2/carbon-appmgt/pull/25
But I didn't add the following changes,
1. Since NetworkUtil class is defined in org.wso2.carbon.utils package
i didn't remove that method
2. That key store thing because it needs to be discussed.
*Sachith Ushan*
Software Engineering intern
WSO2 : http://wso2 <http://goog_1352065705>.com
mobile :- +94 71 8853336
[email protected]
On Fri, Apr 3, 2015 at 8:00 PM, Sachith Herath <[email protected]> wrote:
> 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
> [email protected]
>
> On Fri, Apr 3, 2015 at 1:17 PM, Dinusha Senanayaka <[email protected]>
> 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
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev
