Skip to content

Conversation

mikecdavis
Copy link
Contributor

There were a lot of constructor arguments for the payload models that made reviewing the EventBuilder code difficult to parse. I've converted them all over to builders to help with readability and maintainability going forward.

I also renamed EventBuilder to EventFactory, I think that is more semantically correct (at least to me). Unfortunately, in the overall diff github isn't rendering it as a rename. If you look at the individual commits it does.

@coveralls
Copy link

coveralls commented Jul 27, 2018

Pull Request Test Coverage Report for Build 563

  • 197 of 201 (98.01%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+5.8%) to 89.399%

Changes Missing Coverage Covered Lines Changed/Added Lines %
core-api/src/main/java/com/optimizely/ab/event/internal/payload/Event.java 23 25 92.0%
core-api/src/main/java/com/optimizely/ab/event/internal/payload/Visitor.java 11 13 84.62%
Totals Coverage Status
Change from base Build 551: 5.8%
Covered Lines: 2336
Relevant Lines: 2613

💛 - Coveralls

Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for the refactor!

@mikeproeng37 mikeproeng37 merged commit 714ef5a into master Jul 31, 2018
@aliabbasrizvi aliabbasrizvi deleted the mikecdavis/refactor-with-builders branch August 10, 2018 18:43
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