Skip to content
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

Refactor XMLConfiguration to use Builder Pattern #543

Merged
merged 2 commits into from Jul 24, 2020

Conversation

johnjaylward
Copy link
Contributor

@johnjaylward johnjaylward commented Jul 21, 2020

Fixes #541

What problem does this code solve?

Refactors the XML Configuration to use a builder pattern instead of public constructors with many parameters. This makes viewing the configuration easier to read and maintain.

Risks

The fields on the configuration have changed visibility from public to private and are now accessible via public getters. This means that a new release will not be a drop-in replacement and consumers of this library will need to correct code that is directly referencing those fields. This should hopefully be minimal impact as the fields were readonly (i.e. hopefully no one was actually using them).

Changes to the API?

Yes, public fields were marked private and constructors were deprecated. New methods were put in place for managing the XML Configuration.

Will this require a new release?

Yes

Should the documentation be updated?

I don't believe we have any documentation outside of the Java-docs referencing this API.

Does it break the unit tests?

No, but unit tests were updated to use the new API.

Was any code refactored in this commit?

Yes, as mentioned above.

Review status

APPROVED

John J. Aylward added 2 commits July 21, 2020 11:08
Updates XML configuration to use a builder pattern instead of
constructors with many parameters
@stleary
Copy link
Owner

stleary commented Jul 21, 2020

Starting 3-day comment window.

@johnjaylward
Copy link
Contributor Author

johnjaylward commented Jul 23, 2020

@stleary, would you mind using tags/labels to make it easier to see which PR are up for review and which are ready to commit? Not that we usually have a lot, but it may make it easier to see at a glance from the main PR screen.

@stleary stleary merged commit 7852810 into stleary:master Jul 24, 2020
This was referenced Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

XMLParserConfiguration needs to be opened up.
2 participants