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

Trie stats, query params, and better usage docs #10

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Conversation

BGluth
Copy link
Contributor

@BGluth BGluth commented Jun 10, 2024

For the longest time, trie_stats was living in it's own branch because the changes that it needed upstream was not merged into a crates.io version. Now that a version is published that has these fixes, we can merge trie_stats into main.

The logic for trie queries has always supported different ways to configure the trie output, but it was never accessible by the user. This PR adds prog args that set these.

Also added docs on what format the tries are expected to be in. I really wanted to avoid duplicating this doc 5 times across three binaries, so I ended up settling for using after_about to reuse a long shared common piece of help info. Note that I didn't really want to place this above a single argument since 2/3 of the binaries use this argument twice, which would add a lot of noise to the help page. Ideally, it would be nice to document an argument type and not just an argument, but I don't see this feature at all in clap.

- Added some comments on how to use the tool, specifically regarding
  what format the trie files should be in.
@BGluth BGluth requested a review from Nashtare June 10, 2024 20:15
Copy link
Contributor

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

LGTM!

Cargo.toml Outdated Show resolved Hide resolved
@Nashtare Nashtare added this to the Cleanups and Misc. milestone Jun 13, 2024
@Nashtare Nashtare linked an issue Jun 13, 2024 that may be closed by this pull request
@BGluth
Copy link
Contributor Author

BGluth commented Jun 13, 2024

@Nashtare Not sure how this happened, but a bunch of my changes for query params somehow didn't get into the PR. Added them now, and it should be a quick review.

Copy link
Contributor

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

Yeah the new commit looks good to, just one tiny nit for my OCD

trie_query/src/main.rs Outdated Show resolved Hide resolved
@Nashtare Nashtare merged commit 98d47e7 into main Jun 13, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add trie stats binary
2 participants