search

clojure-emacs / clojure-ts-mode

The next generation Clojure major mode for Emacs, powered by TreeSitter
GNU General Public License v3.0
129 stars 11 forks source link

Setup Eldev, Makefile, Tests and Update Github Actions #35

Closed p4v4n closed 7 months ago

p4v4n commented 7 months ago
bbatsov commented 7 months ago

You should also update the CI github action to use the Makefile.

bbatsov commented 7 months ago

While you're at this you can also add a test folder and one trivial buttercup test, so we'd have the unit test infrastructure in place. (as in clojure-mode)

p4v4n commented 7 months ago

While you're at this you can also add a test folder and one trivial buttercup test, so we'd have the unit test infrastructure in place. (as in clojure-mode)

I have setup 1 unit test with buttercup and added running tests as part of the existing job in CI. Let me know if I update the job name or setup a separate job for tests.

bbatsov commented 7 months ago

I have setup 1 unit test with buttercup and added running tests as part of the existing job in CI. Let me know if I update the job name or setup a separate job for tests.

I was just writing a comment about this. :D For me it's a bit better to have separate actions, as it makes it easier to see what ran/passed/failed in the output.

p4v4n commented 7 months ago

I have setup 1 unit test with buttercup and added running tests as part of the existing job in CI. Let me know if I update the job name or setup a separate job for tests.

I was just writing a comment about this. :D For me it's a bit better to have separate actions, as it makes it easier to see what ran/passed/failed in the output.

Cool. Will setup a new job for tests.

I have a couple of more related questions. 1) What is currently running under lint is actually eldev compile. Should this be updated as well? 2) I am just getting familiar with github actions terminology.Should tests be a separate job inside this workflow/file or a separate workflow? Adding tests as a separate workflow might be costly(assuming each workflow runs on a different VM) once we setup tests on every platform. If we are adding it to the same workflow/file the filename should probably be changed.

Edit: I think I understand what github actions are compared to workflow/jobs now. So tests setup should be in the same file. The 2nd question now is just if I should update the filename as well.

bbatsov commented 7 months ago

What is currently running under lint is actually eldev compile. Should this be updated as well?

Yeah, I think it should just call make lint.

I am just getting familiar with github actions terminology.Should tests be a separate job inside this workflow/file or a separate workflow?

Separate job in the same workflow would be better.

p4v4n commented 7 months ago

Separate job in the same workflow would be better.

I separated compile/lint/test into different jobs.

Current Status: lint is failing as compat package cannot be installed on CI. I have tried a few things but none of them fixed the issue so far.

bbatsov commented 7 months ago

I'm guessing @doublep might have some idea what's going wrong with compat.

p4v4n commented 7 months ago

I'm guessing @doublep might have some idea what's going wrong with compat.

The compat issue was fixed after switching to emacs29.1 instead of snapshot. Likely related to the below news. compat apparently is already part of emacs30 and will not be installed again. https://github.com/emacs-compat/compat/commit/c98e141d14114509caeda4e6752a1aad39a0cdb1

bbatsov commented 7 months ago

Yeah, that sounds like plausible explanation. I had missed this announcement. I guess now all that's left is to fixed the lint errors and squash all the related commits together.

p4v4n commented 7 months ago

I guess now all that's left is to fixed the lint errors and squash all the related commits together.

Fixed the lint errors and cleaned up the commit history.

bbatsov commented 7 months ago

Great work! Thanks!

doublep commented 6 months ago

I'm guessing @doublep might have some idea what's going wrong with compat.

It appears to be an issue in package-lint, not Eldev. Seems to be fixed upstream in this commit, but as Eldev by default issues the latest stable version, you can bump into the bug if using Emacs 30.

To reproduce locally:

$ eldev clean .eldev; eldev docker master -d lint package

inside some project. You can see from the stacktrace that it happens inside package-lint.