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

Upgrade afj #21

Merged
merged 7 commits into from
Aug 3, 2023
Merged

Upgrade afj #21

merged 7 commits into from
Aug 3, 2023

Conversation

mattdean-digicatapult
Copy link
Contributor

This PR is equivalent to openwallet-foundation/credo-ts-ext#222 from the upstream. Most of the comments from there apply equally here. Note I would try to avoid changing too much as the upstream PR is not yet approved and we will likely want to fix this up in line with that.

@mattdean-digicatapult
Copy link
Contributor Author

btw these tests are really flakey and that issue has been raised i nthe upstream

n3op2
n3op2 previously approved these changes Aug 2, 2023
Copy link
Contributor

@dc-andysign dc-andysign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good ... just a few feedback items that perhaps must be addressed ?!

tests/utils/helpers.ts Show resolved Hide resolved
src/cli.ts Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
src/utils/webhook.ts Show resolved Hide resolved
tests/outofband.test.ts Show resolved Hide resolved
tests/proof.test.ts Outdated Show resolved Hide resolved
tests/webhook.test.ts Show resolved Hide resolved
Copy link
Contributor

@dc-andysign dc-andysign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Looks good to me! :)

@mattdean-digicatapult mattdean-digicatapult merged commit 805f066 into main Aug 3, 2023
@mattdean-digicatapult mattdean-digicatapult deleted the upgrade-afj branch August 3, 2023 09:51
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