-
Notifications
You must be signed in to change notification settings - Fork 45
Add error message to catch a plugin being added as a middleware #349
base: master
Are you sure you want to change the base?
Conversation
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.
Need to fix flow error. Also, this is not greenkeeping. I'd say bugfix might be a more appropriate tag. @KevinGrandon I think it's ok to add runtime checks for these, at least for now. We've had a few cases of post-migration code throwing bad errors and wasting people's time that could've been avoided by surfacing errors such as this. Also, the test suite already has a lot of tests similar to this one. |
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.
Ok, works for me then. Requesting changes to clear review queue. Let's get flow issues fixed and request re-reivew. Thanks!
Codecov Report
@@ Coverage Diff @@
## master #349 +/- ##
=========================================
Coverage ? 93.23%
=========================================
Files ? 18
Lines ? 458
Branches ? 92
=========================================
Hits ? 427
Misses ? 17
Partials ? 14
Continue to review full report at Codecov.
|
@KevinGrandon @lhorie please do review ( updated tag to bugfix and flow passes ) |
No description provided.