-
Notifications
You must be signed in to change notification settings - Fork 15
Well documented and more explict code style - particularly in python code #2102
Copy link
Copy link
Open
Labels
Core - based (all apps)buildtaskshttps://github.com/turnkeylinux/buildtaskshttps://github.com/turnkeylinux/buildtaskscloudtaskcommonBugs/feature requests that require changes to shared common build codeBugs/feature requests that require changes to shared common build codeconfconsoledeckdeckdebuilddiscussionebsmountfabTurnKey appliance fabrication tool - preinstalled in TKLDevTurnKey appliance fabrication tool - preinstalled in TKLDevhubclienthttps://github.com/turnkeylinux/hubclienthttps://github.com/turnkeylinux/hubclienthubdnshttps://github.com/turnkeylinux/hubdnshttps://github.com/turnkeylinux/hubdnshubtoolsinit-fenceinithookstkl-installerTurnKey custom ISO installer (replaces di-live): https://github.com/turnkeylinux/tkl-installerTurnKey custom ISO installer (replaces di-live): https://github.com/turnkeylinux/tkl-installertklamqtklbamtkldev-detectiveA tool to lint TurnKey codeA tool to lint TurnKey codetklpatchtodoturnkey-chrootA python chroot library - leveraged by fab-chrootA python chroot library - leveraged by fab-chrootturnkey-gitwrapperpython library for git repo manipulationpython library for git repo manipulationturnkey-sslTurnKey script to generate self signed SSL/TLS certsTurnKey script to generate self signed SSL/TLS certsturnkey-versionTurnKey custom command to provide version related info.TurnKey custom command to provide version related info.
Description
Metadata
Metadata
Assignees
Labels
Core - based (all apps)buildtaskshttps://github.com/turnkeylinux/buildtaskshttps://github.com/turnkeylinux/buildtaskscloudtaskcommonBugs/feature requests that require changes to shared common build codeBugs/feature requests that require changes to shared common build codeconfconsoledeckdeckdebuilddiscussionebsmountfabTurnKey appliance fabrication tool - preinstalled in TKLDevTurnKey appliance fabrication tool - preinstalled in TKLDevhubclienthttps://github.com/turnkeylinux/hubclienthttps://github.com/turnkeylinux/hubclienthubdnshttps://github.com/turnkeylinux/hubdnshttps://github.com/turnkeylinux/hubdnshubtoolsinit-fenceinithookstkl-installerTurnKey custom ISO installer (replaces di-live): https://github.com/turnkeylinux/tkl-installerTurnKey custom ISO installer (replaces di-live): https://github.com/turnkeylinux/tkl-installertklamqtklbamtkldev-detectiveA tool to lint TurnKey codeA tool to lint TurnKey codetklpatchtodoturnkey-chrootA python chroot library - leveraged by fab-chrootA python chroot library - leveraged by fab-chrootturnkey-gitwrapperpython library for git repo manipulationpython library for git repo manipulationturnkey-sslTurnKey script to generate self signed SSL/TLS certsTurnKey script to generate self signed SSL/TLS certsturnkey-versionTurnKey custom command to provide version related info.TurnKey custom command to provide version related info.
The style of existing TurnKey code is inconsistent and often a bit messy. I'd like to have a clear and concise code style "policy" which is well documented somewhere - the
CONTRIBUTING.mdfile in this repo is the obvious place IMO.We've already implemented typing in most (all?) of the TurnKey custom package source code, but I'd also like to update/improve other linting & style "rules".
During my recent work on
tkl-installerI implemented what I believe is a really good style. In particular I'm pretty happy with the detailed doc-strings I implemented. Ideally I'd like to apply that same style across all TKL projects - but I'm open to discussion about exactly what the final "policy" will be.The process of updating all our existing code to comply with the new code "rules" will definitely be a lot of work. But IMO the short term cost will be offset by a significant long term benefit. Even beyond the initial implementation it will likely add a bit of maintenance overhead and a bit of additional friction for new contributors. But OTOH it will make it much easier for future maintainers and contributors to understand what the code does - or at least what it is intended to do. It will also support generation of really extensive and useful docs for all our projects using a tool like
sphinx.Use of tools such as
ruffandmypy- with preset config/rules - will also (hopefully) reduce the risk of bugs, increase opportunities for more automated CI/CD tests and and allow contributors to check their own work before contributing code - or at least a way for contributions that don't comply to be flagged and contributors to be pointed in the right direction. That will reduce the load on us accepting contributions. Obviously we'd still need to do code review, but automated linting, typing and style tests could be run before hand, so a lot of potential issues would hopefully be caught before manual review.Once we agree on the new "policy" we should also ensure that
tkldev-detectiveuses the same rules when linting appliance build code. We also need to add that to the appliance build code update/contribution process - ideally as an automated process that runs on pull requests.shellcheckis fairly straight forward to implement for linting bash scripts - although from my experience to date, making TKL scripts compliant is not always trivial.WRT python code, my preferred ruff config is included in the
tkl-installer/pyproject.tomlfile.I don't really have enough rust knowledge to make any comments on that, but we should probably include rust style guidance too.
Having said all that, right now is obviously not the time to implement all that, but let's have the discussion and at least start thinking about what we want the "policy" to be and how we go about implementing it all.
Related issues:
Related PR: