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.

Sign up for GitHub

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

Jump to bottom

refactor(scaffold): refactor cosmosver and print the migration guide #1550

Merged
merged 35 commits into from
Sep 23, 2021
Merged

refactor(scaffold): refactor cosmosver and print the migration guide #1550

merged 35 commits into from
Sep 23, 2021

Conversation

Pantani
Copy link
Collaborator

@Pantani Pantani commented Sep 7, 2021 •
edited
Loading

close #1548

What does this MR does?

Convert the migration guide to HTML to shows the user how to update any version mismatch.

How to test?

  • Try to scaffold anything into a chain with an SDK version older them v0.43.0 with any scaffold error. You will see the migration message.

  • If you scaffold something into an old chain without errors or into a new chain with errors, the message does not appear.

@Pantani Pantani changed the title feat(cmd): print migration feat(scaffold): print migration guide on version SDK/Starport version mismatch Sep 8, 2021
@Pantani Pantani self-assigned this Sep 8, 2021
@Pantani Pantani marked this pull request as ready for review September 15, 2021 14:59
ilgooz
ilgooz reviewed Sep 16, 2021
View reviewed changes
starport/cmd/scaffold.go Outdated Show resolved Hide resolved
@Pantani Pantani requested a review from ilgooz September 16, 2021 15:52
ilgooz
ilgooz requested changes Sep 17, 2021
View reviewed changes
Copy link
Member

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

This time it'll not show the warning when scaffolding fails. We need to print the warning before starting to scaffolding.

Maybe we can just use a persistent pre run hook for the starport scaffold command?

@Pantani Pantani requested a review from ilgooz September 17, 2021 15:07
@Pantani
Copy link
Collaborator Author

Pantani commented Sep 17, 2021

This time it'll not show the warning when scaffolding fails. We need to print the warning before starting to scaffolding.

Maybe we can just use a persistent pre run hook for the starport scaffold command?

Great, it's done! To put the logic into the PersistentPostRun cobra command, we must pass the app path as a global variable or add it into the cobra context. I put it inside the scaffold.App method because we can avoid to find, read and parse the go.mod twice.

@ilgooz
Copy link
Member

ilgooz commented Sep 17, 2021 •
edited
Loading

We don't necessarily need to use the hook, if that doesn't work, we can just call the check func on each cmd handler.

I think we should avoid printing anything inside services/ to the stdout as much as possible, I would rather keep it in the cmd.

ilgooz
ilgooz previously approved these changes Sep 22, 2021
View reviewed changes
@ilgooz
Copy link
Member

ilgooz commented Sep 22, 2021 •
edited
Loading

I'm always in favor of making things more resilient even if it needs to introduce some limitations feature wise.

If we make the cmd fail in case of a version mismatch, we can also recommend people to use an older, but compatible version of Starport and show downgrade instructions alongside the migration instructions.

@Pantani
Copy link
Collaborator Author

Pantani commented Sep 22, 2021

I'm always in favor of making things more resilient even if it needs to introduce some limitations feature wise.

If we make the cmd fail in case of a version mismatch, we can also recommend people to use an older, but compatible version of Starport and show downgrade instructions alongside the migration instructions.

I like the idea of showing the downgrade instructions, but we do not know whats version the user needs to downgrade to works. We can show a generic guide, but the user needs to discover whats version is

@lumtis
Copy link
Contributor

lumtis commented Sep 23, 2021

Is it still WIP? It's still scaffolding in older chains for me

@fadeev
Copy link
Contributor

fadeev commented Sep 23, 2021

It's easier to update the website, so let's add more content there, instead of the message printed to the terminal.

fadeev
fadeev reviewed Sep 23, 2021
View reviewed changes
starport/cmd/cmd.go Outdated Show resolved Hide resolved
fadeev
fadeev reviewed Sep 23, 2021
View reviewed changes
starport/cmd/cmd.go Outdated Show resolved Hide resolved
ilgooz
ilgooz reviewed Sep 23, 2021
View reviewed changes
starport/cmd/cmd.go Outdated Show resolved Hide resolved
@Pantani Pantani changed the title feat(scaffold): print migration guide on version SDK/Starport version mismatch refactor(scaffold): refactor cosmosver and print the migration guide Sep 23, 2021
@Pantani
Copy link
Collaborator Author

Pantani commented Sep 23, 2021

Is it still WIP? It's still scaffolding in older chains for me

What are you mean? Is the scaffolded version for the chain still 0.42?

here it works well

Screen Shot 2021-09-23 at 12 32 19

@lumtis
Copy link
Contributor

lumtis commented Sep 23, 2021

I mean scaffolding commands still run in chains using 0.42 instead of stopping with the message

@Pantani
Copy link
Collaborator Author

Pantani commented Sep 23, 2021

I mean scaffolding commands still run in chains using 0.42 instead of stopping with the message

Ohh, sorry, I forgot that. I thought I already pushed the code. You can try now

@ilgooz ilgooz merged commit 2d21d80 into develop Sep 23, 2021
@ilgooz ilgooz deleted the feat/print-migration branch September 23, 2021 19:13
@Qdigital Qdigital mentioned this pull request Jul 31, 2022
Open
@vutting4221 vutting4221 mentioned this pull request Jul 31, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Aug 1, 2022
Open
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
@Pantani @ilgooz
refactor(scaffold): refactor cosmosver and print the migration guide (
a34ca52 
ignite#1550)

* print migration tests

* convert entire folder from localfs

* use sub instead create a new embed.FS

* fix linter and migration dir

* fix cmd

* add examples

* improve template parse

* show a warning if cosmos SDK version is too old

* remove html from markdown generation

* add version checker every time a user scaffolds something and there is a version mismatch.

* avoid read parse parse `go.mod` twice

* create newApp method to check the app version

* remove `GetVersion` method and `Export` version field

* create versioning structure

* pass the spinner obj to the newApp method

* fix warning msg condition

* refactor(pkg/cosmosver): refactor (ignite#1607)

* refactor(pkg/cosmosver): refactor

* fix

* Update starport/pkg/cosmosver/cosmosver.go

Co-authored-by: Danilo Pantani <danpantani@gmail.com>

Co-authored-by: Danilo Pantani <danpantani@gmail.com>

* Update starport/pkg/cosmosver/cosmosver.go

* fix ci lint

* update warning message

* return error for version mismatch and remove 0.43.0

* run gofmt

Co-authored-by: İlker G. Öztürk <ilkergoktugozturk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ilgooz ilgooz ilgooz approved these changes

@lumtis lumtis Awaiting requested review from lumtis

@fadeev fadeev Awaiting requested review from fadeev

Assignees

@Pantani Pantani

Labels
None yet
Projects
None yet
Milestone
v0.18
Development

Successfully merging this pull request may close these issues.

Print migration guide on version SDK/Starport version mismatch
4 participants
@Pantani @ilgooz @lumtis @fadeev