-
Notifications
You must be signed in to change notification settings - Fork 28
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
[klippa] subset BASE table #1325
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a few questions inline about whether we need so many traits, but overall this seems reasonable :)
pub use parsing_util::{ | ||
parse_drop_tables, parse_name_ids, parse_name_languages, parse_unicodes, populate_gids, | ||
parse_name_ids, parse_name_languages, parse_tag_list, parse_unicodes, populate_gids, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just noticing now, but are these methods genuinely part of the public API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh this is because I need to use these methods in integration_test, do you know how can I export them to integration_test without making them public? I tried pub(crate) use
but that doesn't work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea integration test only works on public API.
d372ef5
to
f85c1d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, it's kind of annoying to have to pass the empty args in but I think it's an improvement on balance. :)
thanks very much for your valuable review comments! |
No description provided.