Improve Metadata initialization time, add benchmarking, check regexps during build & general housekeeping

[yannleretaille]

Please note that these changes depend on the not yet released version 0.2.1 of regex-cache. This should fix #26 @meh @rustonaut would be great if you guys could have a look!

[yannleretaille]

changed to semantic commit messages

[yannleretaille]

Good points, I'll see what I can do!

On Sun, Oct 18, 2020, 19:34 Philipp Korber [email protected] wrote:

@rustonaut requested changes on this pull request.

I could merge it the way it is and do the noted changes later before publishing a new version. (@meh https://github.com/meh)

In src/lib.rs https://github.com/rustonaut/rust-phonenumber/pull/27#discussion_r507189267 :

@@ -70,7 +71,7 @@ pub use crate::carrier::Carrier; mod phone_number; pub use crate::phone_number::{PhoneNumber, Type};

-mod parser; +pub mod parser;

I guess you made this public because of the benchmark.

But it's not supposed to be part of the public API so it probably should be #[doc(hidden] or similar.

In src/metadata/database.rs https://github.com/rustonaut/rust-phonenumber/pull/27#discussion_r507190331 :

}

/// Create a database from a loaded database.

• pub fn from(meta: Vecloader::Metadata) -> Result<Self, error::LoadMetadata> {

• pub fn from(meta: Vecloader::Metadata, check_regex: bool) -> Result<Self, error::LoadMetadata> {

This is a braking change.

Furthermore the api between from, load and parse is no inconsistent.

If we anyway do a braking change we probably want to have some opaque Options type we can pass into all of from, load and parse.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rustonaut/rust-phonenumber/pull/27#pullrequestreview-511164369, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF3W33JHMVRR35BQ3XVLUTSLMRLJANCNFSM4SSDDMLQ .

[fabricedesre]

@yannleretaille do you think you will have time to resume work on that PR?

[atymic]

Any updates on this pr?

[rustonaut]

I'm closing this as I can't hide it in my Git overview, and it likely won't get any process, and has been superseeded by the fork AFIKT.

[rubdos]

@rustonaut, we don't maintain a fork, you are writing in the live repository.

The easiest way to get it out of your dashboard is probably to reduce your access rights in the repository (if you don't intend do exercise those rights any more), or to unassign issues and PRs.

We initially wanted to keep this open, but @gferon will open an issue to track the follow-up here.

[rustonaut]

Oh, sorry I missed that this is now a PR on whisperfish:main instead of meh:main :see_no_evil: The problem was the reviewed-by filter (so non of the proposed steps would have helped, I would love to not use that filter but due to limitations of other filters and workflows I always end up coming back to it).