Skip to content

Conversation

@pjfanning
Copy link
Member

@pjfanning pjfanning changed the title Module builder Draft: Module builder Jun 25, 2021
@pjfanning pjfanning requested a review from cowtowncoder June 25, 2021 12:55
@cowtowncoder
Copy link
Member

Nothing much to comment since I am not familiar enough to offer much useful feedback.
Just one question: from the way it is passed, the "builder" sounds more like it might be a Configuration/settings object?
If so, maybe it'd be make sense to name it as such? (if I am mistaken about intent, apologies).

@pjfanning
Copy link
Member Author

@cowtowncoder this 'Builder' class follows the Builder pattern and is used to build a ScalaModule (which extends the JacksonModule/Module)

@cowtowncoder
Copy link
Member

@pjfanning Ok then I am confused. From changes it seemed odd that it would be passed to (de)serializers and value instantiators and so on. I'll see if I can grok the flow with more context.

@pjfanning
Copy link
Member Author

I can rename the ReadOnlyBuilder trait as Config - would make that part of the code more readable

@cowtowncoder
Copy link
Member

@pjfanning on builder arg passed, yes, I can see now how it adds things to be added via SetupContext.

@cowtowncoder
Copy link
Member

@pjfanning Ah. On ReadOnlyBuilder as base -- not sure what makes most sense. Given that it really is an internal base type I don't have a strong opinion here.

@pjfanning pjfanning changed the title Draft: Module builder Module builder Jul 17, 2021
@pjfanning pjfanning merged commit 3ded305 into master Jul 17, 2021
@pjfanning pjfanning deleted the module-builder branch July 17, 2021 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants