Skip to content

Fix string parameter bugs#11

Open
danielbosnich wants to merge 8 commits intomasterfrom
fix-default-string
Open

Fix string parameter bugs#11
danielbosnich wants to merge 8 commits intomasterfrom
fix-default-string

Conversation

@danielbosnich
Copy link

@danielbosnich danielbosnich commented Feb 27, 2026

This PR fixes the logic for handling optional string query parameters with a default value. Previously, we would initialize a string value with the default value but then pass it in to a function that was expecting a *string type.

One thing I'm not totally sure about is if we need the dataType function call. That also doesn't work as currently written (since it's not used for the default value) but after some experimentation I'm thinking that this might be dead code. It looks like we don't support default values for string enums, so I'm not sure how this would get executed currently.

@danielbosnich danielbosnich changed the title Fix default string query params Fix string parameter bugs Feb 28, 2026
"net/http"
"net/url"
"os"
"path/filepath"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cordellfreeman, looks like you didn't generate code in #9. That's something we should do, right? Just curious if we're trying to keep these generated files up-to-date.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I probably just forgot. It appears we've done it in all the other PRs so it's good to keep doing

Copy link
Collaborator

@cordellfreeman cordellfreeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what 'dataType` function is or what it does. If it's dead, I think removing it is fine. If somebody does need it, then can add it back easily

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants