emacs-exwm / xelb

X protocol Emacs Lisp Binding
https://elpa.gnu.org/packages/xelb.html
GNU General Public License v3.0
26 stars 4 forks source link

Makefile dependencies and byte-compilation #15

Open medranocalvo opened 7 months ago

medranocalvo commented 7 months ago

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

This is meant to be applied after #4.

Stebalien commented 7 months ago

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

medranocalvo commented 7 months ago

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 commented 7 months ago

We can also remove the byte-compilation rules altogether.

To clarify, I don't mean this sarcastically.

Stebalien commented 7 months ago

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 commented 7 months ago

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

medranocalvo commented 7 months ago

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.

minad commented 7 months ago

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!

medranocalvo commented 7 months ago

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.