Hi Thanuja,
Its okay. Btw, I am seeing small issues in the way we have are logging..
} catch (SQLException e) {
+ log.error("Database error occurred while adding shared
role", e);
throw new UserStoreException(e.getMessage(), e);
} catch (Exception e) {
+ log.error("Error occurred while adding shared role", e);
throw new UserStoreException(e.getMessage(), e);
} finally {
DatabaseUtil.closeAllConnections(dbConnection);
1) Do we really need to log here? Since we are throwing the error to
the calling method, that method can log too. This will cause multiple
levels of logging in the backend.
2) In the log statement, we haven't logged the original error message.
3) In the throw statement, you are not putting the message that your
have in your log statement.
Generally we shouldn't log in every method. Only at the API level or
at the client level.
Thanks,
Sameera.
On Thu, Nov 27, 2014 at 12:14 PM, Thanuja Jayasinghe <[email protected]>
wrote:
> Hi Sameera,
>
> I checked the formatting using Idea before taking the patch for the first
> time. But it looks like there is a bit difference how Idea treat tabs and
> spaces. Sorry about the inconvenience.
>
> Thanks,
> Thanuja.
>
> On Thu, Nov 27, 2014 at 12:05 PM, Thanuja Jayasinghe <[email protected]>
> wrote:
>
>> Hi Manoj,
>>
>> Formatting issues are fixed and diff is attached to [1].
>>
>> [1] - https://wso2.org/jira/browse/IDENTITY-2888
>>
>> Thanks,
>> Thanuja.
>>
>> On Thu, Nov 27, 2014 at 11:15 AM, Sameera Jayasoma <[email protected]>
>> wrote:
>>
>>> Manoj can you please revert this patch. There are some formatting issues
>>> it seems.
>>>
>>> Thanks,
>>> Sameera.
>>>
>>> On Thu, Nov 27, 2014 at 11:04 AM, Manoj Kumara <[email protected]> wrote:
>>>
>>>> Hi Thanuja,
>>>>
>>>> Committed to patch0009 with r209939. Please send the pull request to
>>>> Git repo.
>>>>
>>>> Thanks,
>>>> Manoj
>>>>
>>>>
>>>> *Manoj Kumara*
>>>> Software Engineer
>>>> WSO2 Inc. http://wso2.com/
>>>> *lean.enterprise.middleware*
>>>> Mobile: +94713448188
>>>>
>>>> On Thu, Nov 27, 2014 at 12:09 AM, Thanuja Jayasinghe <[email protected]>
>>>> wrote:
>>>>
>>>>> Hi Carbon Team,
>>>>>
>>>>> Please commit the diff attached with [1].
>>>>>
>>>>> [1] - https://wso2.org/jira/browse/IDENTITY-2888
>>>>>
>>>>> Thanks,
>>>>> Thanuja.
>>>>>
>>>>> --
>>>>> *Thanuja Lakmal*
>>>>> Software Engineer
>>>>> WSO2 Inc. http://wso2.com/
>>>>> *lean.enterprise.middleware*
>>>>> Mobile: +94715979891 +94758009992
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Sameera Jayasoma,
>>> Software Architect,
>>>
>>> WSO2, Inc. (http://wso2.com)
>>> email: [email protected]
>>> blog: http://sameera.adahas.org
>>> twitter: https://twitter.com/sameerajayasoma
>>> flickr: http://www.flickr.com/photos/sameera-jayasoma/collections
>>> Mobile: 0094776364456
>>>
>>> Lean . Enterprise . Middleware
>>>
>>>
>>
>>
>> --
>> *Thanuja Lakmal*
>> Software Engineer
>> WSO2 Inc. http://wso2.com/
>> *lean.enterprise.middleware*
>> Mobile: +94715979891 +94758009992
>>
>
>
>
> --
> *Thanuja Lakmal*
> Software Engineer
> WSO2 Inc. http://wso2.com/
> *lean.enterprise.middleware*
> Mobile: +94715979891 +94758009992
>
--
Sameera Jayasoma,
Software Architect,
WSO2, Inc. (http://wso2.com)
email: [email protected]
blog: http://sameera.adahas.org
twitter: https://twitter.com/sameerajayasoma
flickr: http://www.flickr.com/photos/sameera-jayasoma/collections
Mobile: 0094776364456
Lean . Enterprise . Middleware
_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev
