Skip to content

feat(usage): add usage records anf fix sub item error#195

Closed
pandomic wants to merge 10 commits into
adrienverge:masterfrom
localstack:feat/basic-usage-records
Closed

feat(usage): add usage records anf fix sub item error#195
pandomic wants to merge 10 commits into
adrienverge:masterfrom
localstack:feat/basic-usage-records

Conversation

@pandomic
Copy link
Copy Markdown

@pandomic pandomic commented Sep 6, 2021

  • Fix an error when listing subscription items
  • Add basic implementation for usage records
  • Sync latest changes on LS side

@H--o-l
Copy link
Copy Markdown
Collaborator

H--o-l commented Sep 7, 2021

Hey @pandomic,

Thanks for opening a PR!

A few remarks, however.

There is a lot of commits in the branch you propose, and some look outdated.
Maybe you can rebase and keep only the commits you want this PR to be about?

My second remark is that some of your commits seem to do two things at a time, please split commits per feature or resource or parameters, the level that makes the more sense, and write a commit message as is both simple and enough to understand your goal.
Last things, commits like fix(linter): Fix linter errors which fix things of a previous commit of the same PR should be squashed in the previous commit.

Thanks in advance, cheers!
Hoel

@pandomic
Copy link
Copy Markdown
Author

pandomic commented Sep 7, 2021

@H--o-l thanks for your comments, we cleaned the history and opened a new PR, looking forward: #197

@pandomic pandomic closed this Sep 7, 2021
@pandomic pandomic deleted the feat/basic-usage-records branch September 7, 2021 12: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.

3 participants