-
Notifications
You must be signed in to change notification settings - Fork 63
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
Formatting and linting the codebase #374
Comments
Machinekit-HAL currently has an old This version is explicit (no base config), is based/targeted for version 14 of
With the current codebase, it changes about 480 files. (It will need human interaction, as not all changes are wanted and thus manual adding of |
Pull request #375 implements the basic Mainly, the Github Actions workflow which will check that no unformatted code will be merged into the repository. The second option is to use the pre-commit.ci own CI service which has even option to back-push the required changes to the open pull request, but as of yet I opted to use what is already used and avoid bringing another vendor. (Discussion welcome.) |
I like that. There is a possible complication with parts that are fed into an RTAI kernel module that are in demand of a more ancient formatting style (LinuxCNC/linuxcnc#1575). |
Actually, this is not a problem, because the RTAI was deprecated long ago (before my time), in the original Machinekit/Machinekit repository. Pretty much all modules run in (As far as I know, the main reason for RTAI deprecation were the problems with building, no upstream [meant Debian in this case] kernel packages and general nastiness of RTAI development.) Otherwise, you are right. This work is mostly automatic, because you add the formatter (the repository hooks are usually prepared by somebody else, only the |
Just to clarify, LinuxCNC works quite well as-is with the RTAI 5.3.1 release, andypugh posted debs which have excellent latency. RTAI is not EOL, it is not deprecated, and it still works. 4.19.195 is currently the latest stable kernel tested and confirmed working with LinuxCNC. |
As discussed during the CMake rework, code in the Machinekit-HAL codebase (all Machinekit's codebases, really) should be formatted based on some deterministic rules and then linted for common mistakes and errors.
The changes should happen in two stages:
This should happen for any language (or file) for which the required tools (formatter and/or linter) exist or are easy to develop or are otherwise required and essential for Machinekit-HAL. (Obviously, the easy will be a lot quicker while the hard ones may not even happen.)
From minimal standpoint, the targeted languages should be:
The instrumentation for checking of the proposed changes should be two fold:
git hooks
functionalityThe git hooks option run locally on the developer's side should be very soft requirement. Nobody should be forced to use the predefined workflow, use the pre-selected toolchain or run it at specific times. Given that the end result will the the same, of course. The CI part should be, on the other hard, very hard requirement. Without successful run, the code should not be eligible for merge as defined by C4.
The CI part should be implemented as separate Github Actions (or other free for Open-Source) provider independently from the rest of the testing workflow.
The local git hooks part should be implemented via git hook manager and actionable commands isolated in it's own runtime (script and Docker image, in layman terms). So the action is deterministic and not dependent on any (additional) installed software on the developer's system. Any update to this setup should be manual and definitely not automatic.
To the git hooks manager, I was originally thinking about the Lefthook project as it is simple Go compiled executable (so the runtime is packed inside), however issues with operating on staged files and connected status checking make the usage very hard. (Not unusable, but it is like making you own bread and beating your own butter, when you just want to eat something in the morning before you go to work - death by thousand paper-cuts defined.) So, in the face of this, I have been looking deeper into the pre-commit project. This software is written in Python, thus it requires Python interpreter version higher than
3.6
. It is also failing on the pythonic way of thinking (i.e. Python is a big, bad hammer with which every nail is bashed in, regardless if it was actually a screw), so their idea how to run a clang-format is to wrap it into wheel, but one can work around it with thedocker
approach.The idea is to run the
pre-commit
hook (not the project, idiotic naming) for formatters, so each commit has the right format from the get go. Run the linters as part of thepre-push
hook as that will be time consuming.Work-plan itinerary
clanf-format
rules for C and C++pre-commit
configurationThe text was updated successfully, but these errors were encountered: