feat: add RunFunctionsOptions for server configuration#202
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces RunFunctionsOptions to allow configuring runtime options for runFunctions, specifically the x-powered-by response header. It also adds corresponding unit tests. A review comment suggests removing a redundant import of RunFunctionsOptions in the test file, as it is already available via the direct import of src/server.dart.
demolaf
force-pushed
the
feat/run-functions-options
branch
from
6cd2d20 to
169e443
Compare
| typedef FunctionsRunner = FutureOr<void> Function(Firebase firebase); | ||
|
|
||
| /// Runtime configuration options for [runFunctions]. | ||
| class RunFunctionsOptions { |
There was a problem hiding this comment.
How do we handle this for other SDKs and are we being consistent?
There was a problem hiding this comment.
Other SDKs don't provide support for overriding the default X-Powered-By header, so this is purely an addition to the dart SDK.
Do we want to add support for this?
There was a problem hiding this comment.
I think it's fine. We added default support for x-powered-by to shelf, but folks want to be able to remove it.
|
/gemini review |
kevmoo
left a comment
kevmoo
left a comment
There was a problem hiding this comment.
My only concern is creating a whole new object for ONE property.
Do we think we'll have more in the future?
Would it be better to just add one optional param to runFunctions now and avoid making things complicated until we know we need to?
| typedef FunctionsRunner = FutureOr<void> Function(Firebase firebase); | ||
|
|
||
| /// Runtime configuration options for [runFunctions]. | ||
| class RunFunctionsOptions { |
There was a problem hiding this comment.
I think it's fine. We added default support for x-powered-by to shelf, but folks want to be able to remove it.
There was a problem hiding this comment.
Code Review
This pull request introduces RunFunctionsOptions to allow configuring runtime options for runFunctions, specifically enabling customization of the x-powered-by response header via poweredByHeader. Unit tests were added to verify this behavior. A review comment points out that the test verifying explicit null input for poweredByHeader is identical to the default constructor test, and suggests explicitly passing null to properly test this scenario.
yes this makes sense but |
demolaf
force-pushed
the
feat/run-functions-options
branch
from
c5bd0b5 to
c0fb73e
Compare
4 participants
Closes #120.
Adds a RunFunctionsOptions class to runFunctions, starting with poweredByHeader to allow users to remove or customize the x-powered-by response header.