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

Collector refactoring, part 1 #1435

Merged
merged 9 commits into from
Aug 30, 2022
Merged

Collector refactoring, part 1 #1435

merged 9 commits into from
Aug 30, 2022

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Aug 29, 2022

This is a series of commits that refactor the collector crate in order to make it easier to use it as a library, remove unused code, better structure code to break up large files and to make future integration of runtime benchmarks easier. I will continue with other parts after (if) this gets merged.

Best reviewed commit by commit.

collector/Cargo.toml Outdated Show resolved Hide resolved
@nnethercote
Copy link
Contributor

This mostly seems fine. One suggestion: anything called utils typically ends up as a dumping ground for all sorts of stuff, and might as well be called junk_drawer. Does read2 need to be moved? Could fs be a module at the top-level, next to read2?

@Kobzol
Copy link
Contributor Author

Kobzol commented Aug 30, 2022

I don't like utils.rs files that contain a lot of unrelated things, but I often create utils modules with a file per area of responsibility. A stylistic choice I guess :)

Here I have done it mostly to orient myself in the code. The root contained several files that were intertwined a lot (some of them binaries, some of them included from main.rs, some of them included from lib.rs), so I wanted to group the files into directories to make sense of it. If you don't mind, I would keep it like this until I finish the refactoring, then if you find it clearer I can put the files back into the root.

@Kobzol
Copy link
Contributor Author

Kobzol commented Aug 30, 2022

Rebased on master.

@rylev rylev merged commit 96cae64 into master Aug 30, 2022
@rylev rylev deleted the collector-refactor branch August 30, 2022 16:29
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.

3 participants