PRs quality improvement
This PR should helps to improve the quality in development and future PRs:
- Adding a starting .editorconfig
- Adding MegaLinter as additional PR check. It includes four checks for now:
- EditorConfig, which checks all (applicable) files against the .editorconfig
- Rust Clippy, which using Clippy to check Rust code files
- CSpell, which checks spelling in Markdown files
- Lychee, a link checker written in Rust, which checks local links in Markdown files
The GH-workflow will add a comment to each PR with the status of the checks. For example
Hope this helps to improve quality before a PR is merged. Feedback welcome!
PR Checklist
- [x] fix typos in documentation #920
- [x] fix additional typos #924
- [x] fix CSPELL errors / update word list in cspell.json
- [x] fix (final) line endings / newline
- [x] fix line length
- [x] fix Clippy
Needs #920 be merged first + discussing the errors (especially the setting "max_line_length" in .editorconfig).
Remember to rebase :)
@casperstorm @tarkah what's your favorite line length? Rust coding conventions says 100 (https://rustc-dev-guide.rust-lang.org/conventions.html#line-length). If we use this also, we have to fix some code files (https://github.com/squidowl/halloy/actions/runs/14496743951/job/40666303833?pr=921#step:4:98)
I'm fine with 100.
Conventions say 80 is still preferred. Honestly I have no preference, but keeping things easily reviewable from mobile screens is nice and probably good to keep things at 80 for that reason.
Ok, going with 80. I'll open another PR with the code file fixes...needs some time ;-)
/edit 1789 "violations", mostly line endings :-/
@casperstorm @tarkah I removed Rust files from EditorConfig checks, because we're doing it with rusfmt. With #946 everything should be "green" then and we could integrate this RP.
I think this could be useful, as evidenced by the documentation errors it has already caught. My only concern is whether there is any potential issue with having clippy run twice (by this action and the build action)? I think it could still be useful even if we don't have it run clippy.
Minor notes re: cspell.json:
- I think
vistshould be left off (I think the CHANGELOG entry withvistis a typo ofvisit). - I don't see
pkcsin the documentation so maybe it can be removed as well.
Looks like Megalinter is up to version 8.7.0 now :sweat_smile:
This PR has been very fruitful in consolidating code style expectations and improving the existing GH actions. In particular, PRs #930, #1005, and #1008 owe a great deal to your contributions here. As a result of those PRs, however, the Megalinter checks are almost entirely redundant. Thank you for your help improving the contribution workflow, we greatly appreciate it, but I think we should close this PR in favor of the existing GH actions.