-
Notifications
You must be signed in to change notification settings - Fork 825
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
feat: implement .autoUlid for testing purposes #10442
Conversation
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. |
Hi @danielleadams, I can add it as a PeerDependency, that should work right?
Maybe you can discuss both cases as it would add a newer dependency as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for the contribution @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 I just saw your EDIT by chance ( edits don't trigger email notifications ). Do I need to do something for the rebase? |
@ebisbe the PR needs to be rebased on |
0edf7c6
to
046d9ff
Compare
@danielleadams done |
Thanks @ebisbe - running our test suite on it and then this will be ready to merge. |
@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? |
046d9ff
to
d70c1e5
Compare
015c8ce
to
046d9ff
Compare
@sdstolworthy Merge conflict resolved. |
f6304bc
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
yarn test
passesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.