Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings in tests and compile tests #12

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Hi-Angel
Copy link

This PR implements 3 things:

  1. Fixes all compilation warnings in tests
  2. Makes any byte-compilation warning an error
  3. Makes tests rely on byte-compiled .elc files

Fixes a bunch of warnings like:

    test/test-highlighting.el:1:1: Warning: file has no ‘lexical-binding’ directive on its first line
It's available as part of Emacs distribution since version 24, and
hcl-mode already requires at least 24.3.
The packages initialization is only needed by linting target.
@Hi-Angel Hi-Angel force-pushed the fix-warnings branch 2 times, most recently from c41e60f to bb13e03 Compare February 20, 2024 16:29
@Hi-Angel
Copy link
Author

Hmm… It turns out font-lock-ensure was first defined in Emacs 25, so that makes Emacs 24 tests fail

Fixes warnings:

    In toplevel form:
    test/test-command.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-command.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.

    In toplevel form:
    test/test-highlighting.el:32:7: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-highlighting.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.

    In toplevel form:
    test/test-indentation.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-indentation.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-indentation.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-indentation.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-indentation.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-indentation.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
    test/test-indentation.el: Warning: ‘font-lock-fontify-buffer’ is for interactive use only; use ‘font-lock-ensure’ or ‘font-lock-flush’ instead.
This fixes compilation warnings:

    In end of data:
    test/test-command.el:41:6: Warning: the function ‘forward-cursor-on’ is not known to be defined.
    test/test-command.el:29:4: Warning: the function ‘with-hcl-temp-buffer’ is not known to be defined.

    In end of data:
    test/test-highlighting.el:42:6: Warning: the function ‘forward-cursor-on’ is not known to be defined.
    test/test-highlighting.el:33:16: Warning: the function ‘face-at-cursor-p’ is not known to be defined.
    test/test-highlighting.el:31:6: Warning: the function ‘with-hcl-temp-buffer’ is not known to be defined.

    In end of data:
    test/test-indentation.el:35:6: Warning: the function ‘forward-cursor-on’ is not known to be defined.
    test/test-indentation.el:29:4: Warning: the function ‘with-hcl-temp-buffer’ is not known to be defined.
@Hi-Angel
Copy link
Author

@tgross 24.3 byte-compilation seems to be broken, because it prints warnings about unused values that seem to be completely spurious, the code it complains about does not even have a let binding to begin with.

But I'm wondering, what's the point of having two 24-th versions of Emacs? There're both 24.3 and 24.5. And the latter seems to have the bug fixed. Rather than creating workarounds, I suggest to just drop 24.3 from the testing matrix, what do you think?

Its byte-compilation seems to be broken, because it prints warnings
about unused values that seem to be completely spurious, the code it
complains about does not even have a let binding to begin with.

With that said, the testing matrix already has 24.5, so there's little
point in additionally testing an older Emacs 24.
@Hi-Angel
Copy link
Author

Ping

2 similar comments
@Hi-Angel
Copy link
Author

Hi-Angel commented Jun 4, 2024

Ping

@Hi-Angel
Copy link
Author

Hi-Angel commented Jun 8, 2024

Ping

@Hi-Angel
Copy link
Author

ping 4

@tgross
Copy link

tgross commented Jun 13, 2024

@Hi-Angel please stop pinging to fire notifications. This repo is volunteer-run, and folks will review when they have time.

@Hi-Angel
Copy link
Author

@tgross that's 4 pings in 3 months. Infrequent ping is usually fine and encouraged because people tend to forget stuff.

@Hi-Angel
Copy link
Author

…it is typically fine to ping once a week, so 4 pings in 3 months is very little.

@hcl-emacs hcl-emacs locked as too heated and limited conversation to collaborators Jun 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants