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

Create ignite doctor cmd #3291

Closed
aljo242 opened this issue Dec 14, 2022 · 2 comments · Fixed by #3381
Closed

Create ignite doctor cmd #3291

aljo242 opened this issue Dec 14, 2022 · 2 comments · Fixed by #3381
Assignees
@tbruyelle
Labels
component:cli-doctor Related to Ignite doctor. type:request Feature request.
Milestone
v0.27.0

Comments

@aljo242
Copy link
Contributor

aljo242 commented Dec 14, 2022

Create basic ignite doctor command. This should be hidden originally.

First iteration will:

  • check if tools/tools.go exists
  • populate if it does not
@aljo242 aljo242 added type:request Feature request. component:cli-doctor Related to Ignite doctor. labels Dec 14, 2022
@fadeev
Copy link
Contributor

fadeev commented Dec 15, 2022

What about upgrading config.yml's version? Does this belong in ignite doctor as well, probably not, right? Just trying to find the difference between what we do as part of serve vs doctor. I guess upgrading config.yml is required and doctor performs optional changes.

@tbruyelle
Copy link
Contributor

Good point, this should be included also, will take that into account.

@tbruyelle tbruyelle self-assigned this Dec 15, 2022
@aljo242 aljo242 added this to the v0.27.0 milestone Dec 16, 2022
tbruyelle added a commit that referenced this issue Jan 6, 2023
@tbruyelle
feat: new ignite doctor command
b377200 
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 tbruyelle mentioned this issue Jan 6, 2023
Merged
tbruyelle added a commit that referenced this issue Jan 6, 2023
@tbruyelle
feat: new ignite doctor command
b417d3c 
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
jeronimoalbi added a commit that referenced this issue Apr 6, 2023
@tbruyelle @jeronimoalbi @ilgooz
feat: new ignite doctor command (#3381)
b9ffc43 
* feat: new ignite doctor command

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

* 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]>
Jchicode pushed a commit to Jchicode/cli that referenced this issue Aug 9, 2023
@tbruyelle @jeronimoalbi @ilgooz
feat: new ignite doctor command (ignite#3381)
1418183 
* 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
Assignees

@tbruyelle tbruyelle

Labels
component:cli-doctor Related to Ignite doctor. type:request Feature request.
Projects
None yet
Milestone
v0.27.0
Development

Successfully merging a pull request may close this issue.

feat: new ignite doctor command ignite/cli
3 participants
@tbruyelle @fadeev @aljo242