Feature/get secrets#286
Conversation
|
👋 russell-stern, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
| characterId: string, | ||
| ): StarWarsCharacter => { | ||
| const url = config.url.replace('{characterId}', characterId) | ||
| url = config.url.replace('{characterId}', characterId) |
There was a problem hiding this comment.
I think the intention here was to use url passed in? Right now we are still using URL from config.
Additionally not sure why linter is not picking this but we would like to avoid overriding function params. So the way to this imo would be something like:
const fetchStarWarsCharacter = (
sendRequester: HTTPSendRequester,
config: Config,
_url: string,
characterId: string,
): StarWarsCharacter => {
const url = _url.replace('{characterId}', characterId)
// .......
| characterIds = runtime | ||
| .getSecrets(secretsToFetch) | ||
| .result() | ||
| .map((response: any) => response.response.value.value) |
There was a problem hiding this comment.
The ergonomics seems slightly off here. When you look at regular getSecret when you call .result() on it you just get the value right away. Here user needs to map it himself via accessing response.response.value.value I wonder if we could have getSecrets() return just array of secrets by default?
4 participants
No description provided.