xelb icon indicating copy to clipboard operation
xelb copied to clipboard

Makefile dependencies and byte-compilation

Open medranocalvo opened this issue 1 year ago • 8 comments

I'm just posting the makefile I use to see byte-compilation warnings. Is this

  • The dependencies allow parallel making (make -jN).
  • I like the byte-compilation warnings on xelb-gen. This implies creating the .elc. (Maybe that's not necessary?)

This is meant to be applied after #4.

medranocalvo avatar Jan 18 '24 10:01 medranocalvo

No strong objections, but does elisp-flymake-byte-compile not work for you? Or is this for CI?

Stebalien avatar Jan 18 '24 16:01 Stebalien

I do use it (I actually use an uncommitted Makefile in EXWM as well). I've never triedelisp-flymake-byte-compile, will try to incorporate it.

We can also remove the byte-compilation rules altogether.

I'm undecided on whether lint on CI would be helpful or noisy (we all lint). Now that we have tests it might be worth it.

medranocalvo avatar Jan 19 '24 10:01 medranocalvo

We can also remove the byte-compilation rules altogether.

To clarify, I don't mean this sarcastically.

medranocalvo avatar Jan 19 '24 10:01 medranocalvo

We can also remove the byte-compilation rules altogether.

Remove them from where?

I'm undecided on whether lint on CI would be helpful or noisy (we all lint). Now that we have tests it might be worth it.

IMO, CI lints are nice because:

  1. Sometimes local linting is broken for some reason and, for tiny changes, you may not even notice it.
  2. Sometimes you make a tiny little fix that absolutely shouldn't break anything but does.
  3. It's a simple way to have a first-pass review for contributors.

Not helpful till we get the warnings down to 0, but helpful to keep it there.

Stebalien avatar Jan 19 '24 15:01 Stebalien

(note: I have no objections to this PR if you want to merge it, I'm just pointing out the alternatives)

Stebalien avatar Jan 19 '24 15:01 Stebalien

We can also remove the byte-compilation rules altogether.

Remove them from where?

I meant removing them from the Makefile. But it will be useful for CI (see below).

I'm undecided on whether lint on CI would be helpful or noisy (we all lint). Now that we have tests it might be worth it.

IMO, CI lints are nice because:

1. Sometimes local linting is broken for some reason and, for tiny changes, you may not even notice it.

2. Sometimes you make a tiny little fix that absolutely shouldn't break anything but does.

Oh, it's not just me...

3. It's a simple way to have a first-pass review for contributors.

Not helpful till we get the warnings down to 0, but helpful to keep it there.

I agree on everything. I'm hopeful on the remaining warnings.


I'll try to automatize the discovery of Makefile dependencies if I find some time, to make this more robust. Otherwise I'll merge this.

medranocalvo avatar Jan 19 '24 16:01 medranocalvo

Byte compilation is a good addition to the Makefile. We can also reuse this for CI, where we can compile on various Emacs versions via Steve Purcell's setup-emacs. I am in favor of adding linting, e.g., package-lint. however some linters like Melpazoid (https://github.com/emacs-exwm/xelb/pull/9) go a bit too far imo.

I'll try to automatize the discovery of Makefile dependencies if I find some time, to make this more robust. Otherwise I'll merge this.

Auto generating the dependencies would be great. Thanks!

minad avatar Jan 19 '24 17:01 minad

I'll try to automatize the discovery of Makefile dependencies if I find some time, to make this more robust. Otherwise I'll merge this.

Auto generating the dependencies would be great. Thanks!

Now done. Works on my machine, please test.

It is common to hide the dependency Makefile fragments (.el.d in this patch) as dot-files or in a dot-folder. We can do that if you prefer.

medranocalvo avatar Jan 31 '24 12:01 medranocalvo