Skip to content
This repository has been archived by the owner on Jul 8, 2024. It is now read-only.

Restructure Cadence files #32

Merged
merged 3 commits into from
Feb 23, 2021
Merged

Restructure Cadence files #32

merged 3 commits into from
Feb 23, 2021

Conversation

psiemens
Copy link
Contributor

This PR restructures kitty-items-cadence to be more along the lines of @joshuahannan's proposal here: https://forum.onflow.org/t/typical-dapp-project-structure/814/4?u=pete

This restructure also switches to using filename imports for contracts (instead of placeholders) in preparation for us to integrate the new flow project commands (cc @sideninja).

- Move all files in `contracts`, `transactions` and `scripts` directories
- Use relative file imports in all files (instead of placeholders)
- Clean up comments
- Remove unused/broken scripts or transactions
@joshuahannan
Copy link
Member

Is there a plan to include the templates or contracts package? One of the big reasons for structuring it like this was so that we could have packages for different languages that allow people get automatically construct templates from the application code with the correct import addresses with a import and a single method call. Is that still possible here?

@@ -0,0 +1,199 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

Could we import this from the Flow fungible token repo instead of copying it here?

@@ -1,4 +1,4 @@
import FungibleToken from 0xFUNGIBLETOKENADDRESS
import FungibleToken from "./FungibleToken.cdc"
Copy link
Member

@joshuahannan joshuahannan Feb 22, 2021

Choose a reason for hiding this comment

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

I'd prefer if we could find a way to import contract interfaces without having to copy and paste them to the same repo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, agreed. I left my thoughts here, but I think we could make this work with URL imports: onflow/flow-cli#71 (comment)

@psiemens
Copy link
Contributor Author

Is there a plan to include the templates or contracts package? One of the big reasons for structuring it like this was so that we could have packages for different languages that allow people get automatically construct templates from the application code with the correct import addresses with a import and a single method call. Is that still possible here?

Yep I think this is still possible. This repo didn't have those packages to begin with, but I hope to add them in.

The main change now is the fact that the placeholders (e.g. 0xFUNGIBLETOKEN) don't exist, making it a bit harder to do those address replacements. However, I was thinking we could generalize the code here to handle that.

@10thfloor
Copy link
Contributor

Is there a plan to include the templates or contracts package? One of the big reasons for structuring it like this was so that we could have packages for different languages that allow people get automatically construct templates from the application code with the correct import addresses with a import and a single method call. Is that still possible here?

Yep I think this is still possible. This repo didn't have those packages to begin with, but I hope to add them in.

The main change now is the fact that the placeholders (e.g. 0xFUNGIBLETOKEN) don't exist, making it a bit harder to do those address replacements. However, I was thinking we could generalize the code here to handle that.

Ok, will merge this for now.
@sideninja Is there an issue in the CLI repo for Josh's suggestions?

@10thfloor 10thfloor merged commit a371142 into master Feb 23, 2021
@psiemens psiemens deleted the cadence-restructure branch June 15, 2021 00:05
10thfloor pushed a commit that referenced this pull request Dec 8, 2021
Add item page and style profile pages
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants