Retrofit IOContext with ErrorReportConfiguration
#1068
Conversation
| @@ -141,6 +150,30 @@ public IOContext(StreamReadConstraints src, StreamWriteConstraints swc, BufferRe | |||
| _contentReference = contentRef; | |||
| _sourceRef = contentRef.getRawContent(); | |||
| _managedResource = managedResource; | |||
| _errorReportConfiguration = ErrorReportConfiguration.defaults(); | |||
There was a problem hiding this comment.
Maybe instead call:
this(src, swc, br, contentRef, managedResource, ErrorReportConfiguration.defaults();
There was a problem hiding this comment.
I mentioned above in Notes.1 as...
Use defaults() method and not use new constructor in old constructors
... to minimize changes (git revision), but maybe I have taken it too literally 😅.
Done. Thank you!
| * | ||
| * @since 2.16 | ||
| */ | ||
| public ErrorReportConfiguration getErrorReportConfiguration() { |
There was a problem hiding this comment.
Let's follow the New Jackson Naming convention :)
and drop "get", so errorReportConfiguration() (like streamWriteConstraints())
There was a problem hiding this comment.
Ok, I notice @pjfanning requested getXxx() accessor, so it was "right way" at first. We are moving to reduce use of "get" in cases where there is no existing convention (for particular class) -- get and set method are to be used for general purpose properties/attributes, but not for configuration.
For time being there is obviously some inconsistency: for 3.0 more of get/set prefixes are being/have been removed. But there's no urgency for 2.x.
There was a problem hiding this comment.
hmm, difficult call wrt consistency 🤔 but...
get and set method are to be used for general purpose properties/attributes, but not for configuration.
For this reason, let's go back to original change --one without getXxx()
|
Looks good, added couple of suggestions. You need to merge/rebase from 2.16, I made some changes hopefully easy enough to merge. |
|
Ok, and then one important next step is to wire |
Yes, will be done later today 👍🏻 |
part of #1066
(blocked by #1067)### Notes- As suggested by original PR 1043, trying to minimize changes by
- Introduce "another" new constructor in 2.16(This will keep tests unchanged)
- Use
- After this PR merges, we can do clean ups by
- retrofitting old constructors with new constructor
- Remove the first new constructor in Jackson 2.16 (one without
defaults()method and not use new constructor in old constructorsErrorReportConfiguration