chore: streamline BUILD.bazel file for State Manager #4212
Conversation
stiegerc
changed the title
rs/state_manager/BUILD.bazel
Outdated
| "@crate_index//:strum", | ||
| "@crate_index//:tempfile", | ||
| ], | ||
| deps = DEPENDENCIES + DEV_DEPENDENCIES, |
There was a problem hiding this comment.
It seems that DEPENDENCIES + DEV_DEPENDENCIES introduces unnecessary deps for state_manager_lib_tests and state_manager_integration_test respectively.
There was a problem hiding this comment.
There are pros and cons for grouping dependencies. In the case of "state_manager_lib_tests", which are relatively simple unit tests, I prefer the current way to explicitly list them as there are just a few.
There was a problem hiding this comment.
Went through everything and managed to remove some redundant dependencies. Kept the DEV_DEPENDENCIES though since all the dependencies for state_machine_test are also required for state_machine_integration. There are some extras I listed separately for integration:
deps = [":state_manager"] + DEPENDENCIES + DEV_DEPENDENCIES + [
# Keep sorted.
"//rs/registry/subnet_features",
"//rs/state_machine_tests",
"//rs/test_utilities/io",
"//rs/test_utilities/metrics",
],
But since it also needs quite a few of the DEPENDENCIES and you have to compile :state_manager anyway, I think it's ok to leave it like this right?
Seems reasonable to me, but LMK if you insist on listing it separately. No strong opinion here since it's not that many anyways.
rs/state_manager/BUILD.bazel
Outdated
| "@crate_index//:tempfile", | ||
| ], | ||
| proc_macro_deps = MACRO_DEPENDENCIES + MACRO_DEV_DEPENDENCIES, | ||
| deps = [":state_manager"] + DEPENDENCIES + DEV_DEPENDENCIES, |
|
The renaming part looks good to me as it follows the naming convention inside the monorepo. Thanks. |
It had a target naming scheme at odds with everything else MR related. This has been mildly annoying me for some time.
I modelled it after its counterpart in the replicated state.