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

Improve root module discovery error handling #244

Merged
merged 1 commit into from
Jul 22, 2020

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Jul 22, 2020

This slightly improves logging, where previously we'd log error from plugin cache updating separately and then report 0 errors from loading a module, now all the loading stages are treated the same in terms of error reporting.
The relevant error is however still (intentionally) ignored as watcher invokes these methods:

for _, h := range w.changeHooks {
err := h(ctx, newTf)
if err != nil {
w.logger.Println("change hook error:", err)
}
}

Secondly this also improves the message we show when there is no root module found, to be more helpful until #222 is implemented and reflect that the message is related to the directory, rather than file.

@radeksimko radeksimko requested review from aicarmic and a team July 22, 2020 15:29
@radeksimko radeksimko force-pushed the improve-err-handling branch from d7f3bbf to 9e36a60 Compare July 22, 2020 15:30
@radeksimko radeksimko merged commit 569bedd into master Jul 22, 2020
@radeksimko radeksimko deleted the improve-err-handling branch July 22, 2020 19:48
@ghost
Copy link

ghost commented Aug 21, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the context necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants