Meta: Add makem.sh, et al for linting and CI

[alphapapa]

Hi Omar,

I saw 0d89add and thought that this might help prevent minor oversights like that in the future. Feel free to reject this PR if it's not your style, I'm just presenting it for your consideration. Embark is so useful, that if makem.sh can be helpful to the project, I'd be glad.

Anyway, even if this isn't merged, you might find some of these lint warnings helpful, especially on older Emacs/Org versions, e.g. https://github.com/alphapapa/embark/actions/runs/6203814262/job/16844997873#step:6:157

[oantolin]

Thank you, I really appreciate this. I know I can use help catching silly mistakes like that one you saw, but I've never really had patience for linters. In my very limited experience it seems like it's very hard to write one that doesn't mostly give false positives. For example, here I think that only the thing about org-fold-reveal and maybe package-desc-name are actual problems, in all 400 lines of output. And if I missed something else important, that just proves my point about how hard it is to find the real problems among the false positives.

I guess if you commit to a particular linter you gradually learn how to warp your code so that is both correct and also satisfies the linter, so that you don't have to wade through so much output, right? (Although here I'm not sure how I'd get rid of the incorrect complaints about defvar-keymap. Maybe declaring it as coming from compat?)

Please note that I don't mean to criticize your linter specifically, as this has always been my experience with them and basically the only linter-like things I find myself sticking with are compiler warnings, like gcc -Wall or the byte-compiler in Emacs (which I forgot to run after 0d89add, so I could definitely use some automation!). But maybe this is just because as a non-professional, I've never had to satisfy anyone's code guidelines, and thus haven't seen the benefits of doing so.

I'll definitely think about merging this, after maybe trying it a bit myself. I do use the byte-compiler, checkdoc and package-line-current-buffer (of the 3 the byte-compiler is by far the best, having very close to 0 false positives), but I do not run them automatically and sometimes forget. So maybe I should automate that, at least, byte-compile on buffer save sounds like a good idea.

[alphapapa]

Oh, you're definitely right about linters being noisy and sometimes impractical to satisfy. This is certainly the case here, especially when testing on multiple Emacs versions. It's often impossible to get it to lint cleanly on several versions at once.

I will point out, though, that makem.sh is highly modular and configurable, so if, e.g. you only wanted to run the byte-compile linter, you would just change this line:

https://github.com/oantolin/embark/pull/662/files#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88R73

Instead of lint, call lint-compile (or any combination of the rules, like lint-compile lint-declare, etc).

You can also edit the test.yml file so that the linter will run but not count as a failure in the GitHub Action if it returns non-zero, and then you can just check the job results when you're interested.

Of course, you might also try adding flymake-mode to your emacs-lisp-mode-hook, as it will flag most of the same issues within the buffer. For me, it's easy to overlook those, and I like seeing a project-wide lint/test run in the compilation buffer, so I still use makem before pushing significant changes on my projects.

Regarding the defvar-keymap warnings: makem.sh doesn't implement most of the linters itself; it just wraps them. That warning comes from the package-lint linter (https://github.com/purcell/package-lint); so I guess the the problem is that package-lint isn't accounting for the dependency on compat.

Anyway, the bottom line for me is that these linters help me catch certain kinds of bugs before they reach users (especially bugs regarding supporting multiple versions of Emacs and Org), and therefore before they file a bug report about it. :) But like any automated tool, the results have to be interpreted with a grain of salt.

[minad]

Oh this adds so much code. @alphapapa Could you please turn this into a reusable GitHub action, similar to purcell/setup-emacs@master? I think tarsius also maintains such a reusable action which does linting and which does not duplicate large scripts across repositories. This makes it easier to keep the linter maintained and up to date. I would find a linter setup useful, which I could easily add to many projects by only adding the CI workflows yaml.

[alphapapa]

Oh this adds so much code. @alphapapa Could you please turn this into a reusable GitHub action, similar to purcell/setup-emacs@master? I think tarsius also maintains such a reusable action which does linting and which does not duplicate large scripts across repositories. This makes it easier to keep the linter maintained and up to date. I would find a linter setup useful, which I could easily add to many projects by only adding the CI workflows yaml.

I don't know, maybe. It's always been my intention for makem.sh and its GitHub Actions configuration to be customizeable by each project as-needed, and for it to be upgraded intentionally by the package maintainer. It's always annoying to finally have achieved a clean linting, only for the linting tool to automatically change and not lint cleanly anymore.

The idea is for the package maintainer to be in control, not upstream package maintainers, nor the maintainer of some YAML file (though I greatly appreciate Steve's maintenance of the Emacs setup action, which makem.sh's YAML file uses).

However, when used on GitHub Actions, makem.sh already uses the latest versions of the linting tools each time it runs, and makem.sh rarely changes, so I don't think being outdated is much of a problem.

Of course it would reduce the number of lines added to package repos, but another purpose of makem.sh is to work locally the same way it works on remote CI; it seems like a severe anti-pattern to have to push commits to a remote clone in order to run linters. I'm constantly running it locally, which only takes seconds or less, whereas pushing to GitHub and waiting minutes for an Action to run would be tedious and impractical. It is useful in the way that it makes it easy to lint and test on multiple Emacs versions, rather than having to maintain all of those locally, but for general development, doing so on my local Emacs version or two is enough for me.

So, I'm not opposed to offering a way to run makem.sh as a GitHub Action (and someone already suggested a way to do so with Docker), but I don't know if I will invest the time to do that myself, because I wouldn't use it that way, and I generally encourage package authors to add the script to the repo and customize it if needed (perhaps paradoxically, as I also intend it to be usable without configuration...).

The bottom line, to me, is that it Just Works, so it's not as if it needs to be "maintained," so there's no reason not to add it. (Or if having it in a project root directory seems messy, that will be addressed by alphapapa/makem.sh#40.)

[minad]

Sure. I understand your points. It would just be my preference to not include or even edit any scripts, while I would use some action prepared by the upstream maintainer, in this case by you or Steve. If there is interest I may try to prepare such an action and contribute it to your repository. Regarding breakage I don't worry much. I think GitHub actions have some versioning mechanism too. Configuration could be done via yaml and environment variables, but ideally it would just work. ;)

[minad]

it seems like a severe anti-pattern to have to push commits to a remote clone in order to run linters.

I have my local package-lint-flymake installed in Emacs for direct feedback and I don't need makem.sh for that. Also I don't keep multiple Emacs versions around locally, usually just the current one. Therefore CI for such extended linting adds quite some value, even if it runs only after pushing as a secondary assurance.

[josephmturner]

FWIW, makem.sh can now be used as a git submodule.