castor

Add commons logging to Unmarshaller

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.1.1
  • Fix Version/s: 1.1.2
  • Component/s: XML
  • Labels:
    None
  • Number of attachments :
    5

Description

I have added commons logging to Unmarshaller and UnmarshallHandler, in preparation for adding ErrorHandler to Unmarshaller.

I have run all test cases and everything looks good.

Attachments

  1. Text File
    castor-unmarshaller-patch.txt
    29/Apr/07 1:37 PM
    7 kB
    Paul Philion
  2. Text File
    castor-unmarshallhandler-patch.txt
    29/Apr/07 1:38 PM
    14 kB
    Paul Philion
  3. Text File
    patch.c1957.20070430.txt
    30/Apr/07 1:07 PM
    21 kB
    Werner Guttmann
  4. Text File
    patch.c1957.20070430-002.txt
    30/Apr/07 1:29 PM
    23 kB
    Werner Guttmann
  5. Text File
    unmarshaller-logging-patch.txt
    28/Apr/07 9:19 PM
    21 kB
    Paul Philion

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Werner Guttmann added a comment -

Thanks, Paul. Looks great to me. I have got once concern, though. Removing methods such as setDebug() from both Unmarshaller as well as UnmarshalHandler can cause harm, as these methods are part of a public API. In other words, you cannot really remove them without at least deprecating them for a few months and as such allow people to adjust in a managed manner.As much as I agree that it would be cleaner, I don't think we have an option here.

Show
Werner Guttmann added a comment - Thanks, Paul. Looks great to me. I have got once concern, though. Removing methods such as setDebug() from both Unmarshaller as well as UnmarshalHandler can cause harm, as these methods are part of a public API. In other words, you cannot really remove them without at least deprecating them for a few months and as such allow people to adjust in a managed manner.As much as I agree that it would be cleaner, I don't think we have an option here.
Hide
Permalink
Werner Guttmann added a comment -

Final patch for review.

Show
Werner Guttmann added a comment - Final patch for review.
Hide
Permalink
Paul Philion added a comment -

Good point about the public API. I guess should be them deprecated no-ops.

Should we update docs somewhere to indicate how to get debugging information?

Show
Paul Philion added a comment - Good point about the public API. I guess should be them deprecated no-ops. Should we update docs somewhere to indicate how to get debugging information?
Hide
Permalink
Paul Philion added a comment -

Should I resubmit the patch with the setDebug readded?

Show
Paul Philion added a comment - Should I resubmit the patch with the setDebug readded?
Hide
Permalink
Werner Guttmann added a comment -

Once the patch is in 'committable' state, it should include updates to the docs. And yes, a new patch would be appreciated ?

Show
Werner Guttmann added a comment - Once the patch is in 'committable' state, it should include updates to the docs. And yes, a new patch would be appreciated ?
Hide
Permalink
Paul Philion added a comment -

Updated unmarshaller code that retains setDebug for backwords compatibility.

Show
Paul Philion added a comment - Updated unmarshaller code that retains setDebug for backwords compatibility.
Hide
Permalink
Paul Philion added a comment -

Updated UnmarshallerHandler that contains retains setDebug for compatibility.

Show
Paul Philion added a comment - Updated UnmarshallerHandler that contains retains setDebug for compatibility.
Hide
Permalink
Paul Philion added a comment -

I cannot find which docs to update for Unmarshaller debug. And I didn't want to mess with the JDO/caching docs, where 'debug' seems to be another feature entirely.

Show
Paul Philion added a comment - I cannot find which docs to update for Unmarshaller debug. And I didn't want to mess with the JDO/caching docs, where 'debug' seems to be another feature entirely.
Hide
Permalink
Werner Guttmann added a comment -

Don't worry about the docs right now .... .

Show
Werner Guttmann added a comment - Don't worry about the docs right now .... .
Hide
Permalink
Werner Guttmann added a comment -

Unified patch against src/main/java.

Show
Werner Guttmann added a comment - Unified patch against src/main/java.
Hide
Permalink
Werner Guttmann added a comment -

There's a few areas one needs to look at resolving before committing this code, such as XMLCTF and/or alias, as that's where the old methods most likely will have been used.

Show
Werner Guttmann added a comment - There's a few areas one needs to look at resolving before committing this code, such as XMLCTF and/or alias, as that's where the old methods most likely will have been used.
Hide
Permalink
Werner Guttmann added a comment -

Paul, could I ask you to attach unified patches in the future ? Can I assume that you are working against SVN trunk ? If so, you can simply use 'svn diff' for patch creation.

Show
Werner Guttmann added a comment - Paul, could I ask you to attach unified patches in the future ? Can I assume that you are working against SVN trunk ? If so, you can simply use 'svn diff' for patch creation.
Hide
Permalink
Werner Guttmann added a comment -

Improved patch, cleaning up XML test suite classes accordingly.

Show
Werner Guttmann added a comment - Improved patch, cleaning up XML test suite classes accordingly.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.