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.

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

ScanAll method at FullPrimaryKey, HashPrimaryKey #11

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

extJo
Copy link

@extJo extJo commented Jul 8, 2020

BACKEND-STORY_ID One-sentence summary of changes

Is it a breaking change?: NO

Why did you make these changes?

When scan all records, for iteration, code is so dirty.
So, i think scanAll support at ORM layer.

What's changed in these changes?

I added scanAll method to FullPrimaryKey class.
I added scanAll method to HashPrimaryKey class.

What do you especially want to get reviewed?

Is parameter 'limit' of scanAll method needed?.

Is there any other comments that every teammate should know?

nope.

Submission Type

  • Bugfix
  • New Feature
  • Refactor

All Submissions

  • Have you added an explanation of what your changes?
  • Have you written new tests for your changes, as applicable?
  • Have you checked potential side effects that could make bad impacts to other services?

New Features

  • Have you configured CI/CD properly?
  • Have you configured optimal memory size and timeouts of Lambda Function?
  • Have you grant required permissions (e.g. IAM Policy, VPC Security Group)?
  • Have you made required resources (e.g. DynamoDB Table, RDS Cluster)?
  • Does it requires native addons dependency?

src/query/hash_primary_key.ts Show resolved Hide resolved
src/query/hash_primary_key.ts Outdated Show resolved Hide resolved
src/query/hash_primary_key.ts Outdated Show resolved Hide resolved
@extJo extJo requested a review from breath103 July 9, 2020 07:24
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.

2 participants