Add CONTRIBUTING.md doc #76
Conversation
CONTRIBUTING.md
Outdated
|
|
||
| Note that if you are running with older kernels, you will be missing some | ||
| tests, which could cause a difference between your local testing and the | ||
| github actions CI. The github actions CI tests against all supported kernel |
There was a problem hiding this comment.
I think the claim that github checks all supported ABIs is true, but the github actions here is fairly complicated, so I struggled to fully verify it. Can you double check me on that?
There was a problem hiding this comment.
This is correct, GitHub actions test each commit against each meaningful kernel versions.
This is essentially to make the comment over current_kernel_abi() more visible and add some handholding for new developers. Coming in and running this crate on an older kernel results in multiple test failures, and finding the current_kernel_abi() comment takes a little investigation, so spelling this all out for new contributors may help save some time. Signed-off-by: Daniel Burgener <Daniel.Burgener@microsoft.com>
dburgener
force-pushed
the
contributing
branch
from
71b843a to
a02e33b
Compare
There was a problem hiding this comment.
Thanks for contributing! 🙃
The main issue with this new documentation is that for a change to be fully tested, each commit needs to be tested against all meaningful kernel version, hence the complex GitHub action. We should probably improve landlock-test-tools to easily loop through all the available kernel and run all the tests. For now, the initial kernel are embedded in landlock-test-tools, which is not great, and this was replaced for GitHub action with a more dynamic approach: #72. However, it would require more work to be able to easily run the same tests locally (e.g. building and caching several kernel versions), and it would significantly increase test duration.
By default, if LANDLOCK_CRATE_TEST_ABI is not set, tests assume we are testing the latest kernel version. We should probably change this default behavior to run tests against the version of the running kernel, which will simplify this doc too.
CONTRIBUTING.md
Outdated
|
|
||
| Note that if you are running with older kernels, you will be missing some | ||
| tests, which could cause a difference between your local testing and the | ||
| github actions CI. The github actions CI tests against all supported kernel |
There was a problem hiding this comment.
This is correct, GitHub actions test each commit against each meaningful kernel versions.
If the user doesn't specify a version, run the tests applicable to their actual kernel rather than defaulting to the latest version available. This makes local smoke testing easier. Signed-off-by: Daniel Burgener <Daniel.Burgener@microsoft.com>
Rewrite based on PR comments and change in LANDLOCK_CRATE_TEST_ABI default Signed-off-by: Daniel Burgener <Daniel.Burgener@microsoft.com>
dburgener
force-pushed
the
contributing
branch
from
cd7a936 to
0e5ca97
Compare
|
I pushed an update that tries to address your comment above. Let me know what you think. |
This is essentially to make the comment over current_kernel_abi() more visible and add some handholding for new developers. Coming in and running this crate on an older kernel results in multiple test failures, and finding the current_kernel_abi() comment takes a little investigation, so spelling this all out for new contributors may help save some time.