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

fix: Allow Supply 0 on Master Editions #67

Closed
wants to merge 5 commits into from

Conversation

kespinola
Copy link
Contributor

@kespinola kespinola commented Nov 19, 2021

Issue

The current implementation does not support setting supply to 0 because 0 is falsy in javascript.

Changes

Replace accepting a number as a parameter with BN | null. The caller is responsible for setting it.

package.json Outdated Show resolved Hide resolved
@austbot
Copy link
Contributor

austbot commented Nov 21, 2021

this is a great fix ill bring it in but not the package json changes

@austbot
Copy link
Contributor

austbot commented Nov 21, 2021

Thanks

@austbot
Copy link
Contributor

austbot commented Nov 21, 2021

@kespinola ill fix tests, in a diff branch unless you want to here

@zaxozhu zaxozhu changed the title Allow Supply 0 on Master Editions fix: Allow Supply 0 on Master Editions Nov 22, 2021
kurpav
kurpav previously approved these changes Nov 22, 2021
@kurpav
Copy link
Contributor

kurpav commented Nov 26, 2021

@kespinola there are a few updates that need to be made to fix types form number to BN:

test/actions/mintEditionFromMaster.test.ts
test/actions/signMetadata.test.ts
test/actions/updateMetadata.test.ts

@kespinola
Copy link
Contributor Author

@austbot @kurpav sorry for the crazy late delay. I have adjusted the test cases that were failing. We've also been using this version of the action for some time with no reported problems.

@aheckmann
Copy link
Contributor

Thanks for this. Closing in favor of #147.

@aheckmann aheckmann closed this Jan 13, 2022
@kespinola
Copy link
Contributor Author

Sounds good. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants