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: refactor schema error handling from Go #1308

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

worstell
Copy link
Contributor

  • no longer pass Go compiler errors through the LSP
  • instead of attempting to parse out schema/build errors from an aggregate error, we pass back schema errors and non-schema errors ("real" exceptions) explicitly and separately
  • add errors build file to the ModuleConfig

fixes #1299

@worstell worstell requested a review from a team as a code owner April 19, 2024 00:02
@worstell worstell requested review from deniseli and removed request for a team April 19, 2024 00:02
@worstell worstell changed the title feat: refactor schema error passing from Go feat: refactor schema error handling from Go Apr 19, 2024
@alecthomas alecthomas mentioned this pull request Apr 19, 2024
@worstell worstell force-pushed the worstell/20240418-fix-errors branch from 265c9ba to 9279b61 Compare April 19, 2024 00:03
@wesbillman
Copy link
Member

Screenshot 2024-04-18 at 5 09 32 PM
We might be getting duplicate errors now 🤔

@worstell worstell force-pushed the worstell/20240418-fix-errors branch from 9279b61 to 58502bf Compare April 19, 2024 00:25
@worstell
Copy link
Contributor Author

Screenshot 2024-04-18 at 5 09 32 PM We might be getting duplicate errors now 🤔

fixed!

Comment on lines +38 to +40
if err := os.RemoveAll(filepath.Join(module.AbsDeployDir(), module.Errors)); err != nil {
return fmt.Errorf("failed to clear errors: %w", err)
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

Comment on lines 98 to 100
if len(merr) == 0 {
return
}
Copy link
Member

Choose a reason for hiding this comment

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

Mostly just for my info, but is this check needed? Would just sort.Slice on an empty slice do the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope you're totally right - removed!

- no longer pass Go compiler errors through the LSP
- instead of attempting to parse out schema/build errors from an aggregate error, we pass back schema errors and non-schema errors ("real" exceptions) explicitly and separately

fixes #1299
@wesbillman
Copy link
Member

Screenshot 2024-04-18 at 5 28 43 PM

It's beautiful!! :)

@worstell worstell force-pushed the worstell/20240418-fix-errors branch from 58502bf to 0a46d7a Compare April 19, 2024 00:37
@worstell worstell merged commit 8544aa0 into main Apr 19, 2024
11 checks passed
@worstell worstell deleted the worstell/20240418-fix-errors branch April 19, 2024 00:41
@deniseli deniseli added the approved Marks an already closed PR as approved label Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Marks an already closed PR as approved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

It seems that errors.pb might persist across builds under some circumstances
3 participants