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

Hide full search speed setting from public api #2846

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KyleSiefring
Copy link
Collaborator

Thoughts?

Copy link
Collaborator

@lu-zero lu-zero left a comment

Choose a reason for hiding this comment

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

why do you want it hidden and why aren't you just making the field pub(crate)?

@KyleSiefring
Copy link
Collaborator Author

Mostly want it hidden so that I can add and remove it without multiple api changes.

I felt that using pub(crate) might end up being too error prone. Example: accidentally exposing things by writing pub instead of pub(crate).

One alternative would be to save the speed level and seed internal settings inside the encoder, but not inside api exposed structures.

@lu-zero
Copy link
Collaborator

lu-zero commented Nov 5, 2021

Until I cut a release any API you put move or change is fine and expected to change.

I'll make sure that the non-exhaustive and default() is in place tomorrow.

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