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

Automint - Python library #431

Merged
merged 5 commits into from
Jan 21, 2022
Merged

Conversation

mcvetyty
Copy link
Contributor

@mcvetyty mcvetyty commented Dec 2, 2021

  • Name: Automint
  • Description: A Python library that benefits the token & NFT communities. Scripts allow easy wallet management, automatic creation of unlocked and time-locked policy IDs, as well as the ability to quickly: build, sign, and submit transactions, and much more. Note: This library relies on wrapping cardano-cli. (See Showcase guidelines)
  • Link: <link_to_project>
  • Source: <link_to_source_code>
  • Tags:
    • opensource
    • nativetoken
    • nft

@mcvetyty
Copy link
Contributor Author

mcvetyty commented Dec 2, 2021

@katomm I believe I've successfully done what is needed here. Can you please take a look? Appreciate your help!

@rdlrt I also addressed the three of your comments in this latest pull. Thank you!

@katomm
Copy link
Member

katomm commented Dec 2, 2021

I assume that this replaces #415? Why not just commit to the old PR? :)) It would be easier to keep track of everything.

@katomm katomm added the builder tool Indicates a PR/issue on a builder tool label Dec 2, 2021
@mcvetyty
Copy link
Contributor Author

mcvetyty commented Dec 2, 2021

Hi @katomm Yes - it does replace it. I am sorry, but as I mentioned I am newer to github and this was the only way I could figure out how to make it work. Is it OK? Can you remove pull request 415?

@katomm katomm mentioned this pull request Dec 2, 2021
@mcvetyty
Copy link
Contributor Author

mcvetyty commented Dec 7, 2021

Hi @katomm - Just checking in to see if there's anything further I need to do in order to get this merged?

@katomm
Copy link
Member

katomm commented Dec 9, 2021

@mcvetyty nothing further needed from what I can see. (resolving conflicts would be nice but not a requirement)

@katomm katomm changed the title New branch Automint - Python library Dec 9, 2021
@mcvetyty
Copy link
Contributor Author

mcvetyty commented Dec 9, 2021

@mcvetyty nothing further needed from what I can see. (resolving conflicts would be nice but not a requirement)

Thank you @katomm! Can you tell me then the process for getting it merged? Do I just need to be patient, or do I need to actively try and seek other reviewers? Appreciate your help through all of this.

@mcvetyty
Copy link
Contributor Author

@rdlrt Hi there - are you able to conduct your review on this and get it merged? Please let me know if I need to do anything further..

Copy link
Member

@katomm katomm left a comment

Choose a reason for hiding this comment

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

Like @rdlrt raised in #415 it seems to have stopped support around cardano-node 1.27.0 and while I appreciate that you made a PR there at creativequotient/automint#33, it doesn't look like it will be merged and the use is then questionable as a builder tool at the moment.

@mcvetyty
Copy link
Contributor Author

Like @rdlrt raised in #415 it seems to have stopped support around cardano-node 1.27.0 and while I appreciate that you made a PR there at creativequotient/automint#33, it doesn't look like it will be merged and the use is then questionable as a builder tool at the moment.

I'm sorry, but I am not following. In response to @rdlrt's comments, my team submitted updates to the latest code 22 days ago (version 1.30.1) so that it is current with the Alonzo era and it works perfectly. You can see the updates at the repository:

https://github.com/creativequotient/automint

The pull request to make these updates was #34, not 33: creativequotient/automint#34

Additionally in that update, detailed examples were added. Does that help clarify things?

@katomm
Copy link
Member

katomm commented Dec 23, 2021

Thanks for clarifying, and I apologize if I drew hasty conclusions here. I will look into it.

@mcvetyty
Copy link
Contributor Author

Thanks for clarifying, and I apologize if I drew hasty conclusions here. I will look into it.

Thank you, Tommy! And no need to apologize - I know you all are busy. Just hoping to get this listed before the next CF delegation change.

@rdlrt
Copy link
Collaborator

rdlrt commented Dec 23, 2021

my team submitted updates to the latest code 22 days ago (version 1.30.1)

Sorry but release notes for 1.32.0 has now been consolidated (check breaking changes) - the examples in documentation will be invalid now.

In general tho, my hesitation as a reviewer, especially for builder tools section:

  1. The utility is very narrow/specific scoped to be across builder community.
  2. There was absence of activity for 6 months - across a hard fork and breaking changes for CLI , this did not feel right - as changes required are very miniscule updates to library.

However, please note that my reluctance to give a thumbs up (in its current state as per above) does not prevent your PR from getting merged (as there are plenty of others who may approve if they feel it's appropriate) 👍🏻

All the best 🙂

@katomm katomm merged commit a97b3b2 into cardano-foundation:staging Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builder tool Indicates a PR/issue on a builder tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants