-
Notifications
You must be signed in to change notification settings - Fork 37
Refactored MetricsDefinition to use a MetricBuilder and be based off a super class #149
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
Refactored MetricsDefinition to use a MetricBuilder and be based off a super class #149
Conversation
a63d603
to
6a1c623
Compare
e2a28ae
to
df894a1
Compare
If we want to build the feature first before merging into master, then this PR should point to a feature branch and not master. |
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDefinitionBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/Metric.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/Metric.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java
Outdated
Show resolved
Hide resolved
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java
Outdated
Show resolved
Hide resolved
Tested f59577b on the example lambda and metrics were reported as expected. This commit refactored the previous implementation to now follow the lombok Builder's format as suggested by @markkuhn. However, lombok's |
Tested c3fa3e8:
|
src/main/java/software/amazon/cloudwatchlogs/emf/model/MetricDirective.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Refactored the MetricDefinition to inherit a
Metric
super class which can be inherited by other type of metrics (coming soon). Also refactored MetricDefinition to be immutable and created a MetricDefinitionBuilder which behaves similar to how MetricDefinition used to.Naming is also handled using a setter instead of on creation for simplicity in upcoming metric changes.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.