refactor: remove psp example and improve pop new contract devex #700
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes PSP (Polkadot Standards Proposals) contract templates from the codebase and simplifies the contract generation workflow by eliminating the contract type selection step. Users now directly select from available contract templates.
Key changes:
- Removed PSP22 and PSP34 contract variants from the
Contractenum - Removed the
Pspvariant from theContractTypeenum - Simplified the
NewContractCommandby removing thecontract_typefield - Refactored the interactive prompting logic to be inline within the
execute()method - Updated test generation to work without contract type specification
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/pop-contracts/src/templates.rs | Removed PSP contract type and PSP22/PSP34 template variants, updated tests |
| crates/pop-contracts/src/new.rs | Removed special handling for PSP template extraction logic |
| crates/pop-cli/src/commands/new/contract.rs | Removed contract_type field, refactored prompting to be inline, simplified contract generation |
| crates/pop-cli/src/commands/new/mod.rs | Updated Command::Contract instantiation to remove contract_type field |
| crates/pop-cli/tests/contract.rs | Simplified test to generate templates without contract type specification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #700 +/- ##
==========================================
- Coverage 76.99% 76.92% -0.07%
==========================================
Files 111 111
Lines 25357 25244 -113
Branches 25357 25244 -113
==========================================
- Hits 19524 19420 -104
+ Misses 3765 3764 -1
+ Partials 2068 2060 -8
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Daanvdplas
force-pushed
the
daan/refactor-remove_psp_examples
branch
2 times, most recently
from
1ec4ef6 to
19155f7
Compare
There was a problem hiding this comment.
Nice!
I was initially hesitant about removing the types, but after some testing I like it, it’s definitely cleaner this way.
Regarding the PSP templates, we usually deprecate the template first and remove them in the next release (e.g #628).
However, after checking the metrics, it seems the PSP templates haven’t been used for quite a while, so it’s cleaner to remove them all at once.
Edit
I see the test test_contract_in_workspace_with_simple_name is failing after change the way to prompt.
To fix just add here https://github.com/r0gue-io/pop-cli/blob/main/crates/pop-cli/src/commands/new/contract.rs#L351 the standard template to avoid prompting the user:
let cli = Cli::parse_from([
"pop", "new", "contract", "flipper", "--template", "standard"
]);
|
The removal was based off feedback from users at the PBA. The removal of psp standards was simple, no one indeed uses them and they are not needed anymore with Eth compatibillity. |
|
@AlexD10S curious why that failing test doesn't show up in the CI except in the coverage job, that should not be the case right? |
Yes. We cleaned up the CI pipeline. Coverage already runs the unit tests, so we removed the separate unit test job we had before. Tests now run only once in the coverage job. |
Hmm oke, I didn't find the CI output that clear wasn't able to find error wihthout using copilot. Will look if I can improve it. |
Daanvdplas
force-pushed
the
daan/refactor-remove_psp_examples
branch
from
19155f7 to
452a301
Compare
Closes #553
Also: