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

Added option of only using Account Address in some methods #113

Merged

Conversation

Dhruv-Mishra
Copy link
Contributor

@Dhruv-Mishra Dhruv-Mishra commented Jan 7, 2025

Adopting suggestions from from #67

Since TS Types and Interfaces do no exist at runtime, added a custom Typeguard for Account Object Verification. (Reference: https://www.typescriptlang.org/docs/handbook/advanced-types.html)

Alternatively, if modification to the definition Account is allowed, a custom tag or a new property can be added to the account to verify it's type.

Ran npm test, along with self written unit tests to verify that all related tests pass successfully.

@Dhruv-Mishra Dhruv-Mishra marked this pull request as ready for review January 7, 2025 20:40
@kopy-kat
Copy link
Member

kopy-kat commented Jan 8, 2025

looks good - could you please make sure the tests are passing before me merge

@Dhruv-Mishra
Copy link
Contributor Author

Dhruv-Mishra commented Jan 8, 2025

I did verify the tests locally with the 'npm test' command:
image

Added a commit for requeuing the build, awaiting workflow approval

@Dhruv-Mishra Dhruv-Mishra requested a review from kopy-kat January 8, 2025 17:55
@kopy-kat
Copy link
Member

kopy-kat commented Jan 8, 2025

it looks like it fails on pnpm build

@kopy-kat
Copy link
Member

kopy-kat commented Jan 8, 2025

(see the gh action)

@Dhruv-Mishra
Copy link
Contributor Author

Yes, just checked the logs, will push a commit to fix this

@Dhruv-Mishra
Copy link
Contributor Author

Updated the index to include the newly defined function, awaiting workflow approval

@kopy-kat
Copy link
Member

kopy-kat commented Jan 8, 2025

still failing - you should be able to repro locally by running pnpm build

@Dhruv-Mishra
Copy link
Contributor Author

still failing - you should be able to repro locally by running pnpm build

The build now works fine locally, awaiting workflow approval

@kopy-kat
Copy link
Member

kopy-kat commented Jan 8, 2025

lgtm

@kopy-kat kopy-kat merged commit e822263 into rhinestonewtf:main Jan 8, 2025
1 check failed
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