Skip to content
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 support for optional LANG param #18

Merged
merged 1 commit into from
Jan 25, 2021
Merged

Add support for optional LANG param #18

merged 1 commit into from
Jan 25, 2021

Conversation

timurgarif
Copy link
Contributor

#16

@alexisvisco
Copy link
Member

Hey thanks for your pull request, can you provide a go enum of the language supported by sonic please ?
It will be nicer for the developers :)

@timurgarif
Copy link
Contributor Author

Oh, good point! I didn't think of it. The only drawback of this approach would be need to maintain the enum every time Sonic adds/removes support for new languages.
I'll do that a little bit later.

Also the PR introduces the public API change. The package has no releases yet (no stable sem versions). So it should be OK?

@alexisvisco
Copy link
Member

It's not a big deal, developers need to be agile, I am not active on the project so there are some consequences haha :)

@alexisvisco
Copy link
Member

For maintaining the enum not really, you can override or add an enum since it's a string.

@alexisvisco
Copy link
Member

If you really want to not introduce a breaking change you can add a Query() without the lang and a QueryWithLang()
Not optimal but it's a solution.

@alexisvisco alexisvisco added the enhancement New feature or request label Jan 25, 2021
@timurgarif
Copy link
Contributor Author

Nice. Then we'd better to stick to the current API to keep it closer to the original Sonic protocol.

BTW, I amened the PR as you suggested.

@alexisvisco alexisvisco merged commit a5da12c into expected-archives:master Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants