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.

Sign up for GitHub

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

Jump to bottom

SummaryBuilder #108

Merged
merged 2 commits into from
Feb 8, 2017
Merged

SummaryBuilder #108

merged 2 commits into from
Feb 8, 2017

Conversation

jamlen
Copy link
Member

@jamlen jamlen commented Jan 31, 2017 •
edited
Loading

Closes #104

  • Add SummaryBuilder
  • Move JSONSummaryBuilder and YAMLSummaryBuilder to their package folders under serialisation
  • Add some extra test coverage
  • Refactored
  • Ready for review

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (develop@8477899).

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8477899...618b711. Read the comment docs.

@jamlen jamlen changed the title Moving CreateSummary to an instance method on AggregatorSnapShot SummaryBuilder Jan 31, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+1.5%) to 55.947% when pulling 618b711 on SummaryBuilder#104 into 8477899 on develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.8%) to 55.259% when pulling d4e8ee9 on SummaryBuilder#104 into 8477899 on develop.

reaandrew
reaandrew approved these changes Feb 8, 2017
View reviewed changes
Copy link
Member

@reaandrew reaandrew left a comment

Choose a reason for hiding this comment

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

Approved. We should double check using the word 'this' in the context but from the perspective of disabling any warnings as I agree with the approach. We have not had a linter blow up over it so...yeh. All good! 👍

@reaandrew reaandrew merged commit 26c99b0 into develop Feb 8, 2017
@reaandrew reaandrew deleted the SummaryBuilder#104 branch February 8, 2017 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reaandrew reaandrew reaandrew approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jamlen @codecov-io @coveralls @reaandrew