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.

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

feat: improve analyzer errors #2743

Merged
merged 9 commits into from Jun 21, 2023
Merged

Conversation

jorgeepc
Copy link
Contributor

@jorgeepc jorgeepc commented Jun 15, 2023

This PR improves the implementation of Analyzer Errors by evolving the current errors string array to a proper struct that can handle future enhancements like suggestions, error level, expected values, etc.

type Error struct {
	Error       string   `json:"error"`
	Value       string   `json:"value"`
	Expected    string   `json:"expected"`
	Level       string   `json:"level"`
	Description string   `json:"description"`
	Suggestions []string `json:"suggestions"`
}

Changes

  • Update the errors string array to a struct
  • Improve error messages from rules
  • Group error messages in the webApp

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

Loom

https://www.loom.com/share/134da01378e54eae922298297f4fca9c?sid=a6484e4a-aa09-45f4-a0a9-b38df3e3b186

@jorgeepc jorgeepc self-assigned this Jun 15, 2023
@jorgeepc jorgeepc changed the title feat: add analyzer grouped errors feat: improve analyzer errors Jun 19, 2023
@jorgeepc jorgeepc marked this pull request as ready for review June 20, 2023 18:17
Copy link
Collaborator

@xoscar xoscar left a comment

Choose a reason for hiding this comment

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

Hey @jorgeepc great job! I added a couple of questions, let me know what you think

level:
type: string
description:
type: string
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we include a field for the documentation URL? or is it your idea that it could be part of the suggestions field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can decide that later. Now that we have the Error struct we can increment the fields without breaking the compatibility.

web/src/constants/Analyzer.constants.ts Outdated Show resolved Hide resolved
web/src/services/Span.service.ts Outdated Show resolved Hide resolved
@jorgeepc jorgeepc merged commit 9b4f226 into main Jun 21, 2023
24 checks passed
@jorgeepc jorgeepc deleted the 2677-add-analyzer-grouped-errors branch June 21, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Analyzer] UX - Error message improvement
2 participants