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

show the error output from validatorLogs func in lov viewer #98

Merged
merged 4 commits into from
May 5, 2022

Conversation

SvenDowideit
Copy link
Member

closes #41

@nathanleclaire
Copy link
Contributor

side note on this one -- is there a reason you decided to go for console.log instead of logger.error on these error logs?

@nathanleclaire
Copy link
Contributor

very nice! appreciate this one as it's been a thorn in my side for ages.

have a couple lint issues build is complaining about, but once those are fixed, lgtm

@SvenDowideit
Copy link
Member Author

SvenDowideit commented May 4, 2022

I'm going to claim the console.log thing is because #94 isn't merged yet :) (yeah, and i realise how dumb that is - its 100% why i wanted the same UX on main and renderer)

wrt the throw Error - exceptions are ... for exception cases, not for normal, detected results - and this is not exceptional.

@nathanleclaire
Copy link
Contributor

wrt the throw Error - exceptions are ... for exception cases, not for normal, detected results - and this is not exceptional.

either/or, if it's ok with lint, fine by me, just seem to recall my VSCode complaining about it

@nathanleclaire
Copy link
Contributor

nathanleclaire commented May 4, 2022

now that #94 is in business, lgtm

@SvenDowideit
Copy link
Member Author

yeah, that linter error was weird - mixing return string and return { output } - for now i'm going with just returning a string, and showing that in the UI - atm some point, I hope to upgrade the ipc code to something typed - that thunking makes me cringe like its 1996

@SvenDowideit SvenDowideit merged commit 8add1a8 into main May 5, 2022
@SvenDowideit SvenDowideit deleted the improve-exec-error-UX branch May 5, 2022 00:05
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.

Need to display error when Docker is not detected
2 participants