-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#815] Indicate that JAXB is optional #854
Conversation
Signed-off-by: Andy McCright <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot fasttrack Spec and JavaDoc changes according to our committer rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of questions. You guys are the experts, so I'll leave it to you on how to answer them. Otherwise, looks good!
also include those for JSON. For more information about these providers | ||
see <<jsonp>> and <<jsonb>>. | ||
also include those for JSON or XML. For more information about these providers | ||
see <<jsonp>>, <<jsonb>> and <<jaxb>>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the oxford comma after <<jsonb>>
, but maybe that's just me... :-)
@mkarg - you're right. no fast track on this one. Thanks for pointing it out. You have requested changes, but I didn't see any comments indicating what you'd like to see changed. @kwsutter I don't have a strong opinion about the extra comma - I've seen it both ways (with and without a comma before the last object). If you're ok with it, I'll leave it as-is unless there are other changes that need to go in. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The requested change was to not fast track this PR. I removed that phrase from the description.
@andymc12 Why not merging this one? |
Modifications to the spec doc and javadoc that indicate that JAXB is optional. Where possible, I tried to keep the language consistent with JSON-P / JSON-B - i.e. if the product supports JSON-B, then it must have a built-in entity provider to handle JSON requests - likewise for JAXB / XML.
This change is targeted for the
master
branch (3.0), and should also go into the3.1-SNAPSHOT
branch.Fixes #815