feat: new ignite doctor command #3381
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix #3291 - add ignite doctor command (hidden for the moment) - migrate config.yml if needed - create tools/tools.go if not present, then install dependency tools. The `tools/tools.go` scaffold is handled by the same generator as `scaffold chain`, with an update allowing it to filter the fileset. `xgenny.NewEmbedWalker`, which was used to remove the `files/` prefix, is replaced by `fs.Sub` from the standard library. - update `cosmosgen.InstallDepTools()` - remove retro compatibilty with chain that doesn't have `tools/tools.go`, in other words, remove the call to `go get` - run `go mod tidy` to update `go.sum` - suggest to run ignite doctor when dependency tools install fails - move `GOSUMDB=off` inside `gocmd.ModTidy` instead of setting it prior to the call. This ensures it's always set. Because the command uses intensively the filesystem, I chose to test it with integration tests only. The integration test introduces a new library called `testscript` [0], which is very convenient for this kind of tests. `testscript` is based txt files, where each file is a test case that contains some instructions. The txt file for `ignite doctor` runs `ignite doctor` and then asserts what has changed in the filesystem, according to some commands like `cmp`. The initial state before the command is run is also specified in the txt file, with a list of files designated by `-- filename --` followed by the file content. [0] https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript
tbruyelle
requested review from
aljo242,
lumtis,
fadeev,
jeronimoalbi and
Pantani
as code owners
tbruyelle
force-pushed
the
feat/doctor_cmd
branch
from
b377200 to
b417d3c
Compare
jeronimoalbi
reviewed
Jan 6, 2023
|
There's an issue with the network integration: the test runs I see 2 solutions:
Any other solution ? |
aljo242
reviewed
Jan 6, 2023
Let's discuss in the community call? |
Pantani
reviewed
Jan 9, 2023
|
Hey guys, this PR should be merged: I got the confirmation it fixes an annoying downgrade issue. The PR is stuck because of a problem with spn, but I noticed that the removal of the network command is on the roadmap. So let's also remove/comment the |
ilgooz
previously approved these changes
Mar 10, 2023
ilgooz
reviewed
Mar 10, 2023
ilgooz
reviewed
Mar 10, 2023
ilgooz
reviewed
Mar 10, 2023
There was a problem hiding this comment.
Great!
@jeronimoalbi can you please review / merge? Thanks.
ilgooz
reviewed
Mar 10, 2023
|
LGTM 👍 Once the issue with the tests and the conversations are resolved we can merge. |
Pantani
reviewed
Mar 13, 2023
Co-authored-by: Jerónimo Albi <[email protected]>
jeronimoalbi
approved these changes
Apr 6, 2023
Jchicode
pushed a commit
to Jchicode/cli
that referenced
this pull request
Aug 9, 2023
* feat: new ignite doctor command Fix ignite#3291 - add ignite doctor command (hidden for the moment) - migrate config.yml if needed - create tools/tools.go if not present, then install dependency tools. The `tools/tools.go` scaffold is handled by the same generator as `scaffold chain`, with an update allowing it to filter the fileset. `xgenny.NewEmbedWalker`, which was used to remove the `files/` prefix, is replaced by `fs.Sub` from the standard library. - update `cosmosgen.InstallDepTools()` - remove retro compatibilty with chain that doesn't have `tools/tools.go`, in other words, remove the call to `go get` - run `go mod tidy` to update `go.sum` - suggest to run ignite doctor when dependency tools install fails - move `GOSUMDB=off` inside `gocmd.ModTidy` instead of setting it prior to the call. This ensures it's always set. Because the command uses intensively the filesystem, I chose to test it with integration tests only. The integration test introduces a new library called `testscript` [0], which is very convenient for this kind of tests. `testscript` is based txt files, where each file is a test case that contains some instructions. The txt file for `ignite doctor` runs `ignite doctor` and then asserts what has changed in the filesystem, according to some commands like `cmp`. The initial state before the command is run is also specified in the txt file, with a list of files designated by `-- filename --` followed by the file content. [0] https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript * fix lint * chore: rename xfilepath.Must * Update ignite/pkg/gocmd/gocmd.go Co-authored-by: Jerónimo Albi <[email protected]> * Update ignite/services/doctor/doctor.go Co-authored-by: İlker G. Öztürk <[email protected]> * fix lint --------- Co-authored-by: Alex Johnson <[email protected]> Co-authored-by: Jerónimo Albi <[email protected]> Co-authored-by: İlker G. Öztürk <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #3291 #3268
config.ymlif neededtools/tools.goif not present, then install dependency tools.The
tools/tools.goscaffold is handled by the same generator asscaffold chain, with an update allowing it to filter the fileset.xgenny.NewEmbedWalker, which was used to remove thefiles/prefix, is replaced byfs.Subfrom the standard library.cosmosgen.InstallDepTools()tools/tools.go, in other words, remove the call togo getgo mod tidyto updatego.sumGOSUMDB=offinsidegocmd.ModTidyinstead of setting it prior to the call. This ensures it's always set.Because the command uses intensively the filesystem, I chose to test it with integration tests only. The integration test introduces a new library called
testscript[0], which is very convenient for this kind of tests.testscriptis based on txt files, where each file is a test case that contains some instructions. The txt file forignite doctorrunsignite doctorand then asserts what has changed in the filesystem, according to some commands likecmp. The initial filesystem state before the command is run is also specified in the txt file, with a list of files designated by-- filename --followed by the file content.[0] https://pkg.go.dev/github.com/rogpeppe/go-internal/testscript