Make builders and static ctors consistent #1596
Conversation
| StaticTableSchema.builder() | ||
| .newItemSupplier(Record::new) | ||
| .attributes( | ||
| fromString("id", Record::getId, Record::setId).as(primaryPartitionKey()), |
There was a problem hiding this comment.
I'd like to have some discussion about whether this is semantically correct and what standard we should use here. Not necessarily disagreeing, just keeping this comment as a placeholder to make sure that discussion happens offline.
| @@ -52,7 +52,7 @@ public void generateTransactWriteItem() { | |||
| .expressionValues(singletonMap("key2", stringValue("value2"))) | |||
| .build(); | |||
| ConditionCheck<FakeItem> operation = | |||
| ConditionCheck.of(Key.of(stringValue(fakeItem.getId())), conditionExpression); | |||
| ConditionCheck.create(Key.of(stringValue(fakeItem.getId())), conditionExpression); | |||
There was a problem hiding this comment.
Shouldn't it be Key.create?
dagnir
force-pushed
the
mapper-builders
branch
from
cd25746 to
553230a
Compare
Codecov Report
@@ Coverage Diff @@
## master #1596 +/- ##
============================================
+ Coverage 75.7% 75.71% +<.01%
+ Complexity 684 682 -2
============================================
Files 907 905 -2
Lines 28385 28383 -2
Branches 2255 2255
============================================
+ Hits 21488 21489 +1
+ Misses 5876 5875 -1
+ Partials 1021 1019 -2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I think it looks good. Just one non-blocking thing, I do think the asString etc. is better than fromString or the original string, but I wonder if this is something we should just take a quick moment to poll the team and vote on a few alternatives and see if anyone has an even better idea.
|
@bmaizels Sure will do |
dagnir
force-pushed
the
mapper-builders
branch
from
43b99e8 to
6272280
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
…c8b8c8d9d Pull request: release <- staging/2a71d721-9cc1-4cb0-a733-a6ec8b8c8d9d
Description
Make builders and static ctors consistent. Also removed the
builder()methods from interfaces.Motivation and Context
Consistent with the rest of the SDK and the our standards: https://github.com/aws/aws-sdk-java-v2/blob/master/docs/design/FavorStaticFactoryMethods.md#favor-static-factory-methods-over-constructors
Testing
mvn clean install -pl :dynamodb-enhanced -amScreenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsLicense