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

Dang/add tokenfactory module #1025

Closed

Conversation

GNaD13
Copy link
Contributor

@GNaD13 GNaD13 commented Sep 29, 2022

add module tokenfactory

@codecov
Copy link

codecov bot commented Sep 29, 2022

Codecov Report

Merging #1025 (2974771) into main (a76999a) will increase coverage by 0.10%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1025      +/-   ##
==========================================
+ Coverage   59.68%   59.79%   +0.10%     
==========================================
  Files          52       52              
  Lines        6586     6599      +13     
==========================================
+ Hits         3931     3946      +15     
+ Misses       2367     2366       -1     
+ Partials      288      287       -1     
Impacted Files Coverage Δ
app/app.go 89.21% <100.00%> (+0.30%) ⬆️
x/wasm/keeper/keeper.go 88.07% <0.00%> (+0.31%) ⬆️

@alpe
Copy link
Contributor

alpe commented Sep 30, 2022

Thank you for your PR!
This code looks like a version that you copied from osmosis.
Although I do understand the demand to have this code available, we should work on a process that includes the Osmosis team as code owners and maintainers.
Please contribute to #911 to discuss and follow up on this.

@alpe alpe closed this Sep 30, 2022
@GNaD13
Copy link
Contributor Author

GNaD13 commented Sep 30, 2022

Thank you for your PR! This code looks like a version that you copied from osmosis. Although I do understand the demand to have this code available, we should work on a process that includes the Osmosis team as code owners and maintainers. Please contribute to #911 to discuss and follow up on this.

Thank you for your reviews! @alpe

@faddat
Copy link
Contributor

faddat commented Oct 1, 2022

Hi guys, the osmosis team is well aware that we are using and promoting the token factory code in the ecosystem.

In fact just about everybody that we work with is really keen on this particular pull request

Okay so let me tell you a little bit about the motivation behind this pull request and also tell you like we can maintain it we work with Osmosis we spoke to like everybody about this pull request and basically everybody wants to have the token factory Juno wants to have the token factory, Eve wants to have the token factory, neutron even wants to have the token factory and it just seems an extremely sensible standard.

And yes, it was absolutely copied from osmosis, so that contracts written for osmosis could be compatible in more places that was the explicit intent

@alpe
Copy link
Contributor

alpe commented Oct 6, 2022

@faddat thanks for the additional context! I appreciate the effort and like us to continue the discussion on the issue.
A PR is often pushing a concrete solution while the problem description and motivation can be more inspiring and helpful to come up with different solutions. In this case though it is more about go.mod vs embeding and prioritization of course.

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