search

duraspace / lambdora

Fedora Repository API implemented with AWS Lambda, API Gateway and DynamoDB
Apache License 2.0
9 stars 3 forks source link

Adds testing for DynamoDB interaction #30

Closed bbranan closed 7 years ago

tdonohue commented 7 years ago

👍 Looks good overall. I did add a minor comment about refactoring the tests, but it's not necessarily required.

Another minor thing...this PR seems to have a bunch of unrelated commits listed (8 commits in total). I suspect this could be cleaned up by a rebase...or we just "squash" this during the merge.

bbranan commented 7 years ago

@tdonohue Tests have been split out.

The issue with this PR is that I neglected to start a new branch when doing this work. I was still working on the dynamodb branch which was merged and deleted by the previous PR. I did a merge from master to resolve conflicts, then created this PR. I noticed then, as you did, that the PR includes a larger set of commits than I expected. I tried a rebase on master, but that didn't do the trick either. Squashing the commits on merge is likely the best option.

tdonohue commented 7 years ago

👍 Looks good. I think this is ready to merge