Hello all, The gist is in the subject.
## Rationale The main rationale is that we want to reduce friction when working on the code base, and, more importantly, when submitting and reviewing patches. Pointing out basic style errors costs time and energy on both sides. If we can reduce that, the time saved can be better spent looking at semantic changes. Additionally, various tools and editors do not work well with mixed Tab/space indentation because it's rarely used and doesn't provide any real benefit. Since we're switching our master branches to build for the Proxmox releases based on Debian Trixie, now is a good time to implement this change, as we can more easily backport fixes when doing so before switching over. Note that, although this change was rolled out swiftly, it was considered during the last two major releases. Back then, the effort was hindered by the inability to find a code formatter and/or configuration that closely matched our desired style. Additionally, such things won't always improve on their own, and even if all the tools were ready, it would still require significant effort to drive the process forward. At this point, I'd like to give a shout-out to the main perltidy developer, who quickly fixed several bugs, implemented new features based on our feedback, and clarified some confusion on my end due to the vast flexibility and options of perltidy. Being able to quickly add new options without breaking such a complex Perl project is no small feat. Many thanks also to Wolfgang and Dominik. Wolfgang helped me test earlier versions and was the only one who responded to my request for feedback on Perl Tidy. Dominik quickly tested Biome and came up with a package and config that we can use as a starting point. He's already considering some options we could enable in the future. ## Used Formatters While we still don't have the perfect choice yet, perfection is definitely the enemy of good in this case, and the situation has improved quite a bit. Our choice of formatters will be: ### Perl [perltidy], with a custom perltidyrc file. This file is used by default by a small proxmox-perltidy wrapper that we provide as a package in our devel repository. Our configuration is very close to the desired style guide. When the output is odd, the input uses poor style patterns that cannot be fixed by formatting alone. [perltidy]: https://perltidy.sourceforge.net/ ### JavaScript [Biome], also with a custom config and wrapper available as proxmox-biome package. The biggest difference between what we want and what we get is an extra space before the keyword/name and the parentheses for the function's arguments on function declarations. This could be patched relatively easily. However, as this behavior seems common in the JS world—Biome copied it from Prettier, a widespread JS formatter—I'm fine with adopting it. For what it's worth, we also plan to replace ESLint, which is currently used for linting JavaScript, with Biome to avoid requiring multiple tools. [Biome]: https://biomejs.dev/ ## Drawback for Inflight Patches The biggest drawback is that inflight patches currently need to be rebased on top. This creates some churn, but it's unavoidable if we ever want to change the status quo. Much of the churn can be reduced by using the "--ignore-space-change" flag with "git am" or "git rebase" commands. _However_, for those rebasing or applying patches on top of a tidied repository: Please re-run the tidier on every commit and amend the changes when doing so. This could look something like: git rebase -i --ignore-space-change --exec "make tidy && git add -A && git commit --amend --no-edit" origin/master If there's no tidy target you can use the tool directly, e.g., to apply it to all .pm and .pl (Perl) files known to git at a specific revision one could use: `git ls-files ':*.p[ml]'| xargs proxmox-perltidy` instead of `make tidy`. It's naturally advised to recheck the commit changes and retest the entire branch afterward. An alternative is to merge commits from staff repositories if they save time and effort. For example, one could run a formatter on the feature branch before merging it, and then handle any extra conflicts in the merge commit itself. Additionally, one might be able to use Git's "rerere" functionality to resolve conflicts more quickly. ## Timeline This change starts basically now. IMO, there isn't much benefit to have a longer heads-up here. Naturally, it would have been slightly nicer if it had happened closely after a point release, when open patches are normally at their lowest count. However, we weren't ready back then, so now it is. I'll go through the repositories one by one, starting with the Perl code bases. This process will probably take one or two days, as I will likely evaluate and incorporate a few other changes, as I do with every major release. Please feel free to reply to this thread with suggestions on how to deal with conflicts on inflight patches/commits or with any other questions about this topic. - Thomas _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
