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

style: naming examples #79

Closed
wants to merge 4 commits into from
Closed

style: naming examples #79

wants to merge 4 commits into from

Conversation

Daanvdplas
Copy link
Collaborator

There was a warning in compiling a contract when adding pop api dependency. This should resolve it.

I also updated the naming, which we can use to move forward with the examples.

This is just naming things, examples need a lot more work.

@Daanvdplas Daanvdplas requested review from al3mart and peterwht April 22, 2024 09:09
@evilrobot-01
Copy link
Collaborator

pop-api is included in the path, so having it in the contract names seems redundant. Simpler to just remove the pop_api_ prefixes imo.

@Daanvdplas Daanvdplas requested a review from brunopgalvao April 22, 2024 12:43
@Daanvdplas Daanvdplas force-pushed the daan/fix_example_names branch from f77f2bb to 5c00397 Compare April 22, 2024 13:35
Copy link
Contributor

@brunopgalvao brunopgalvao left a comment

Choose a reason for hiding this comment

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

In general, I would remove the pop_api_ or PopApi prefix and just name it according to the name of the top level folder.

Similar to https://github.com/paritytech/ink-examples

pop-api/examples/balance-transfer/lib.rs Outdated Show resolved Hide resolved
pop-api/examples/balance-transfer/lib.rs Outdated Show resolved Hide resolved
pop-api/examples/nfts/lib.rs Outdated Show resolved Hide resolved
pop-api/examples/nfts/lib.rs Outdated Show resolved Hide resolved
pop-api/examples/balance-transfer/lib.rs Outdated Show resolved Hide resolved
pop-api/examples/balance-transfer/Cargo.toml Outdated Show resolved Hide resolved
pop-api/examples/balance-transfer/lib.rs Outdated Show resolved Hide resolved
pop-api/examples/nfts/lib.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@al3mart al3mart left a comment

Choose a reason for hiding this comment

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

Looks good.
CI is failing but haven't spent much time figuring why

@Daanvdplas Daanvdplas force-pushed the daan/fix_example_names branch from a890520 to dd2c836 Compare April 29, 2024 19:33
@Daanvdplas
Copy link
Collaborator Author

Closed by #86

@Daanvdplas Daanvdplas closed this May 5, 2024
@Daanvdplas Daanvdplas deleted the daan/fix_example_names branch May 5, 2024 10:02
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.

4 participants