-
Notifications
You must be signed in to change notification settings - Fork 40
Description
Context
In Line 23 CONTRIBUTING.md is referenced but doesn't seem to exist. How about adding one and referencing it in various places including the project's README in the Contribution section?
Let me know if you'd like to to help create this. I've added some thoughts here to start the conversation - you know the project and the codebase far better than I do and I expect you've ideas and plans to improve it that I'm not currently aware of.
Rustfmt
Presumably it'd include running rustfmt as illustrated in the CI workflow:
rustfmt --edition 2024 $(git ls-files '*.rs')
BTW this command hangs silently if there are no files in the commit that match *.rs which may happen occasionally e.g. if a commit only has markdown files. Something like:
files=$(git ls-files '*.rs')
if [ -z "$files" ]; then
echo "No Rust files found"
exit 2
fi
rustfmt --check --edition 2024 $files
exit $?
or
files=$(git ls-files '*.rs'); if [ -z "$files" ]; then exit 2; else rustfmt --check --edition 2024 $files; fi
may be more robust? (I've asked it to return 2 to make this condition explicit here, however you might prefer it to return 0 and write a log message such as:
files=$(git ls-files '*.rs'); [ -z "$files" ] && { echo "No Rust files found, skipping rustfmt"; exit 0; } || rustfmt --check --edition 2024 $files
More tests please?
Code contributions might benefit from additional automated tests. If so, what sort of guidance would help contributors? I'd suggest a good start is helping them know how to run the current automated unit tests and the cargo fuzz tests.
cargo test --all-features -p holo-bfd -p holo-bgp -p holo-isis -p holo-ldp -p holo-ospf -p holo-rip -p holo-vrrp
and
fuzz/fuzz-all.sh
What makes tests valuable and maintainable? Where would you like them adding in the codebase e.g. on a per routing protocol basis?
Testing holo as a system?
Perhaps this is outside the scope of this request, nonetheless if contributors are able to run holo and test it as a system that may help increase the confidence their contributions don't break holo (and might even improve its behaviour :) )
Out of date guides?
I found https://github.com/holo-routing/holo/wiki/Holo-Guide-%E2%80%90-Setting-up-dependencies partly as I was trying to work out why the fuzz test targets don't compile on one of my ubuntu machines where I'm building holo for the first time. (The compilation error is rust-lld: error: unable to find library -lyang but the details are out-of-scope for this ticket). Anyway, is that guide still accurate? e.g. it includes cloning libyang then building and installing it locally.