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

SqliteReportBuilder: pull rng from test_utils instead of dependency injection #33

Closed
wants to merge 2 commits into from

Conversation

matt-codecov
Copy link
Collaborator

@matt-codecov matt-codecov commented Sep 6, 2024

i don't really like this approach so this is more of an RFC. now that it's written i'm kind of thinking about giving up on controlling the RNG to begin with though haha

currently parse_pyreport() takes an output file. i want to expose to Python a version that doesn't which will create a new db in std::env::temp_dir(). since parse_pyreport() and SqliteReportBuilder already have multiple entrypoints to allow controlling the RNG, this is making things messier

the other approach is something like

PyreportParser::new()
    .with_report_json(f1)
    .with_chunks(f2)
    .with_rng(StdRng::from_entropy())
    .with_outfile(out)
    .parse()

but then to use the same approach with SqliteReportBuilder you need a SqliteReportBuilderBuilder which is silly


Stack created with Sapling. Best reviewed with ReviewStack.

Copy link

codecov bot commented Sep 6, 2024

❌ We are unable to process any of the uploaded JUnit XML files. Please ensure your files are in the right format.

@matt-codecov
Copy link
Collaborator Author

yeah this doesn't feel right, i am going to try again

@matt-codecov matt-codecov deleted the pr33 branch September 13, 2024 19:34
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.

1 participant