Details
- Reviewers
ngie - Group Reviewers
network tests - Commits
- rG65f065ec7131: netlink: connect netlink tests to the build
rG3873bdc2f28f: netlink: connect netlink tests to the build
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
The change looks ok, based on the diff I see.
A word of warning though: using ctypes is inherently “dangerous” in the sense that it’s easy to crash the interpreter since ctypes is using cpython in (as the .NET CLR referred to it), “unmanaged mode”. It might make a ton of sense to add CFFI wrappers to clean up the C/cpython interface and to make it less crash prone with invalid input.
Hot take: the PEP8 tools used by phabricator should be updated to ignore inline type hints. It might also be a good idea to update the config to work with black’s defaults (<=88 columns).
| tests/atf_python/utils.py | ||
|---|---|---|
| 2 | It would be nice if this used a more portable idiom. | |
| 29 | Using latin-1 or ascii instead of utf-8 might be a good idea, given that the modfind API only works with iso8991 character sets. | |
Thank you for the review!
Hot take: the PEP8 tools used by phabricator should be updated to ignore inline type hints. It might also be a good idea to update the config to work with black’s defaults (<=88 columns).
Would be really nice, though I'm not sure where do we store the configs.