amplify-cli icon indicating copy to clipboard operation
amplify-cli copied to clipboard

Published 20 hours ago • verified publisher icon aws-amplify

feat: implement .autoUlid for testing purposes

Open ebisbe opened this issue 2 years ago • 7 comments

Description of changes

Add $util.autoUlid() function for Velocity template testing purposes.

Issue #, if available

Description of how you validated changes

Check the response has a Ulid format. Currently 26 characters from the spec

Checklist

  • [x] PR description included
  • [x] yarn test passes
  • [x] Tests are changed or added
  • [ ] Relevant documentation is changed or added (and PR referenced)
  • [ ] New AWS SDK calls or CloudFormation actions have been added to relevant test and service IAM policies

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ebisbe avatar May 19 '22 14:05 ebisbe

Hey @ebisbe - thanks for the contribution! Since this pull request is introducing a new dependency, we need to discuss with the team. I will keep you posted with updates.

danielleadams avatar Jun 27 '22 16:06 danielleadams

Hi @danielleadams,

I can add it as a PeerDependency, that should work right? Also I wanted to add the new

$util.autoKsuid() : String
Returns a 128-bit randomly generated KSUID (K-Sortable Unique Identifier) base62 encoded as a String with a length of 27.

Maybe you can discuss both cases as it would add a newer dependency as well.

ebisbe avatar Jun 27 '22 16:06 ebisbe

@ebisbe running our test suite on this and the I can approve. Thanks for your patience here! (This might need to be rebased because we changed some packages names, but lets see if it works)

UPDATE: can confirm it needs a rebase.

danielleadams avatar Jul 22 '22 16:07 danielleadams

@danielleadams I just saw your EDIT by chance ( edits don't trigger email notifications ). Do I need to do something for the rebase?

ebisbe avatar Jul 25 '22 06:07 ebisbe

@ebisbe the PR needs to be rebased on dev.

danielleadams avatar Jul 25 '22 16:07 danielleadams

@danielleadams done

ebisbe avatar Aug 03 '22 15:08 ebisbe

Thanks @ebisbe - running our test suite on it and then this will be ready to merge.

danielleadams avatar Aug 08 '22 16:08 danielleadams

@ebisbe Following up, it looks like there are a few merge conflicts that need to be resolved. We'd love to get this merged in. Would you mind tagging one of us once you have those resolved and we'll work to get this merged?

sdstolworthy avatar Dec 16 '22 17:12 sdstolworthy

@sdstolworthy Merge conflict resolved.

ebisbe avatar Dec 21 '22 15:12 ebisbe