-
-
Notifications
You must be signed in to change notification settings - Fork 644
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
feat(powered-by): optional server name #3492
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3492 +/- ##
==========================================
- Coverage 95.58% 95.57% -0.02%
==========================================
Files 155 155
Lines 9357 9357
Branches 2730 2737 +7
==========================================
- Hits 8944 8943 -1
- Misses 413 414 +1 ☔ View full report in Codecov by Sentry. |
Hi @PatrickJS Thank you for the PR. This is good! It is a new feature but we can introduce it in the next patch release because it's not such a big change. There should be tests. Can you add tests? |
I think making the option as an object, not a string, is good. Like the following: diff --git a/src/middleware/powered-by/index.ts b/src/middleware/powered-by/index.ts
index caa3be7c..4db425f9 100644
--- a/src/middleware/powered-by/index.ts
+++ b/src/middleware/powered-by/index.ts
@@ -5,9 +5,13 @@
import type { MiddlewareHandler } from '../../types'
-export const poweredBy = (serverName = 'Hono'): MiddlewareHandler => {
+type PoweredByOptions = {
+ serverName?: string
+}
+
+export const poweredBy = (options?: PoweredByOptions): MiddlewareHandler => {
return async function poweredBy(c, next) {
await next()
- c.res.headers.set('X-Powered-By', serverName)
+ c.res.headers.set('X-Powered-By', options?.serverName ?? 'Hono')
}
} |
ok made the changes thanks |
Thanks! Can you add a test? |
yup just did 🫡 |
done done |
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.
LGTM!
Thanks @PatrickJS ! Looks good. I'll merge this later and will include this change in the next patch release. It's a new |
* refactor(powered-by): optional server name * style(powered-by): format * test(powered-by): test new config * Update index.test.ts
The author should do the following, if applicable
bun run format:fix && bun run lint:fix
to format the code