Test and implementation for warning on missing Beacon.LiveAdmin.Plug in the :browser pipeline #75
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n Beacon.LiveAdmin.Plug is missing from the :browser pipeline. Closes BeaconCMS#71.
|
I guess I should reference this closes Issue #71 in this thread. |
leandrocp
requested changes
Sep 18, 2023
There was a problem hiding this comment.
Hey @kgautreaux thanks for the PR!
I'm not sure this is the best testing strategy but it does have the advantage of real world verisimilitude.
The test is perfect.
I'm also not sure that rescuing the error and reraising with a logger warning
Left a code suggestion.
Please feel free to squash the commits if this can be merged or let me know and I can squash and resubmit the PR.
You can push as many commits as you want, I always squash before merging so don't worry about it.
leandrocp
approved these changes
Sep 20, 2023
There was a problem hiding this comment.
Hey @kgautreaux only format is failing but the implementation looks good so I'm merging this one. Thank you!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm not sure this is the best testing strategy but it does have the advantage of real world verisimilitude.
I'm also not sure that rescuing the error and reraising with a logger warning explaining the fix is emphatic enough, but I thought I'd give it a try since the guides rewrite will probably prevent someone from forgetting to add this step.
Please feel free to squash the commits if this can be merged or let me know and I can squash and resubmit the PR. Thanks!