-
Notifications
You must be signed in to change notification settings - Fork 31
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
Hook errors#665 #666
Hook errors#665 #666
Conversation
Codecov Report
@@ Coverage Diff @@
## master #666 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 54 54
Lines 2840 2849 +9
Branches 534 538 +4
=========================================
+ Hits 2840 2849 +9
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
const hookResults = await this.config.runHook('pre-deploy-event-reg', { appConfig: config }) | ||
if (hookResults?.failures?.length > 0) { | ||
// output should be "Error : <plugin-name> : <error-message>\n" for each failure | ||
this.error(hookResults.failures.map(f => `${f.plugin.name} : ${f.error.message}`).join('\nError: '), { exit: 1 }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to extract this into a function in the class? Since it is duplicated thrice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its not exactly the same all 3 times
if we need to write it 5 times I'll consider it, or if it expands into more than 1 line.
LGTM |
Closes #665
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: