Skip to content

feat!: add explicit command to fix target health issues#261

Merged
federicobozzini merged 10 commits into
mainfrom
fix-command
May 8, 2026
Merged

feat!: add explicit command to fix target health issues#261
federicobozzini merged 10 commits into
mainfrom
fix-command

Conversation

@federicobozzini
Copy link
Copy Markdown
Contributor

Changes

  • added new fixCommand property to the output result of the health check. This property doesn't appear in plain output, only json.

Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
@federicobozzini federicobozzini requested a review from a team as a code owner May 7, 2026 13:01
@muchzill4 muchzill4 changed the title feat: added explicit command to fix target health issues feat: add explicit command to fix target health issues May 7, 2026
Copy link
Copy Markdown
Contributor

@muchzill4 muchzill4 left a comment

Choose a reason for hiding this comment

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

There's a lot of duplication between Fix and FixCommand, can you think of a way where we could use FixCommand as the source of truth for the command, and Fix only for the meta text?

Something like (pseudo-template):

{{Issue}}
   {{Fix}}
   {{If FixCommand}}
     -> run `{{FixCommand}}`

@federicobozzini
Copy link
Copy Markdown
Contributor Author

There's a lot of duplication between Fix and FixCommand, can you think of a way where we could use FixCommand as the source of truth for the command, and Fix only for the meta text?

This was also my first thought but:

  • often fixes are more than just run '<fixCommand>'.
  • some issues don't have a fixCommand associated, fix is just a generic suggestion.

I think this is one of those cases where removing dupilications makes things messier rather than cleaner.

Comment thread internal/health/dependencies.go Outdated
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Comment thread internal/health/health.go Outdated
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Copy link
Copy Markdown
Contributor

@muchzill4 muchzill4 left a comment

Choose a reason for hiding this comment

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

Thanks for introducing the nilable Fix and tweaking descriptions all over the shop. This is going to be much better than what we had. 🦾

Comment thread internal/health/dependencies_test.go Outdated
Comment thread internal/health/health_test.go Outdated
Comment thread internal/output/templates/health_test.go Outdated
Comment thread e2e/setup_keys_test.go
Comment thread internal/health/health.go Outdated
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Signed-off-by: Federico Bozzini <federico.bozzini@arm.com>
Copy link
Copy Markdown
Contributor

@muchzill4 muchzill4 left a comment

Choose a reason for hiding this comment

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

Needs marking as breaking change and LGTM. Nice work.

@federicobozzini federicobozzini changed the title feat: add explicit command to fix target health issues feat!: add explicit command to fix target health issues May 8, 2026
@federicobozzini federicobozzini merged commit 8899114 into main May 8, 2026
6 checks passed
@federicobozzini federicobozzini deleted the fix-command branch May 8, 2026 15:20
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.

2 participants