-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add option to suppress adapter feature support log #13842
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
Add option to suppress adapter feature support log #13842
Conversation
🦋 Changeset detectedLatest commit: 7fbd8a6 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CodSpeed Performance ReportMerging #13842 will not alter performanceComparing Summary
|
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.
Thank you @yanthomasdev! This is a new feature, so we need to add a minor changeset. If you really want you could direct this PR to the cloudflare one and do everything in one go. The next minor is in two weeks, so we have time
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.
This PR is blocked because it contains a minor changeset. A reviewer will merge this at the next release if approved.
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.
Just a few typos in docs, otherwise all good
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.
Two tiny suggestions for the changeset @yanthomasdev , but the messages look fantastic!
Co-authored-by: Sarah Rainsberger <[email protected]> Co-authored-by: Matt Kane <[email protected]>
Changes
This adds a new
suppressoption to thesupportedAstroFeaturesofsetAdapterin core. You can either set"all"to suppress both the default and custom log, or"default"to only suppress the default log.The idea is that integration authors can suppress the log if a feature is supported in a specific condition, or if the default log can be conflicting and confuse users. You can take #13817 as an example of when this happens.
Originally, I intended to make
suppressa boolean option while also changing so you only log the custom message if provided, but doing so would imply a breaking change (I think?). So I went with this solution.Testing
Added a new fixture:
packages/astro/test/fixtures/feature-support-message-suppresionNot sure how I'd go about testing this automatically, but manually:
pnpm install pnpm build cd packages/astro/test/fixtures/feature-support-message-suppresion pnpm devYou should see that the dev server has only a "This should be logged." warning from
[adapter]. You can see in the fixture that there's an example of both using asuppress: "default"andsuppress: "all".Docs
PR: withastro/docs#11825