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

Replace cl by cl-lib #2802

Merged
merged 7 commits into from
Jun 13, 2021
Merged

Replace cl by cl-lib #2802

merged 7 commits into from
Jun 13, 2021

Conversation

stephan-cr
Copy link
Contributor

Since Emacs 27 the package cl is deprecated, the replacement is cl-lib, which is available since Emacs 24.3.

This patch replaces cl by cl-lib and drops support for Emacs versions less than 24.3. Dropping older Emacsen is required, because cl-lib is a builtin starting from version 24.3 and doesn't need an extra package from ELPA.

Testcases for past issues (in test/issues) still contain cl. Most of them seem to be broken and need further investigation.

This patch is tested with test/run-ert.sh, which outputs:

Ran 10 tests, 10 results as expected, 0 unexpected (2021-01-30 13:24:54+0100, 0.672122 sec)
1 expected failures

and manually by daily usage for a month now.

This PR addresses issue #2792.

@stephan-cr
Copy link
Contributor Author

stephan-cr commented Jun 13, 2021

Rebased to current master. Some check for Emacs 24.5 fails with fatal: origin/master...HEAD: no merge base. I have to figure out why.

@tarao
Copy link
Collaborator

tarao commented Jun 13, 2021

Aha, I guess it is about a workaround replacing TRAVIS_COMMIT_RANGE. I will fix it but it may take some time.

@stephan-cr
Copy link
Contributor Author

@tarao It must be something different, the message fatal: origin/master...HEAD: no merge base appears for the successful ones as well.

@tarao
Copy link
Collaborator

tarao commented Jun 13, 2021

@stephan-cr I think I could fix it now. It seems that byte-compilation tests should run if there is change outside recipe/.

Here are results from a test pull request; I think these are what you have expected:
https://github.com/dimitri/el-get/pull/2838/checks?check_run_id=2813750015

@stephan-cr
Copy link
Contributor Author

stephan-cr commented Jun 13, 2021

@tarao For some strange reason https://github.com/dimitri/el-get/pull/2838/checks?check_run_id=2813750015 gives:

In toplevel form:
methods/el-get-go.el:42:43:Error: reference to free variable ‘el-get-dir’

I cannot reproduce it locally, but I fix that nevertheless.

Edit: I can, with:

$ byte-compile -Werror methods/el-get-go.el

In toplevel form:
methods/el-get-go.el:42:43:Error: reference to free variable ‘el-get-dir’

seems like byte-compile -Werror *.el methods/*.el is not reliable.

@stephan-cr
Copy link
Contributor Author

@tarao Fixed all compilation errors. The PR doesn't look nice, because it fixes several unrelated errors, due to unreliable CI. I created a follow-up issue to fix the remaining compilation errors.

Since Emacs 27 the package cl is deprecated, the replacement is
cl-lib, which is available since Emacs 24.3.

This patch replaces cl by cl-lib and drops support for Emacs versions
less than 24.3. Dropping older Emacsen is required, because cl-lib is
a builtin starting from version 24.3 and doesn't need an extra package
from ELPA.

Testcases for past issues still contain cl. Most of them seem to be
broken and need further investigation.

This patch is tested with test/run-ert.sh, which outputs:

Ran 10 tests, 10 results as expected, 0 unexpected (2021-01-30 13:24:54+0100, 0.672122 sec)
1 expected failures

and manually by daily usage for a month now.
The function mapcan exists outside cl since Emacs 26.1.
This change is required, because Emacs 23 doesn't natively provide
cl-lib.
Emacs 28 enforces 80 characters per doc string line.
Copy link
Collaborator

@tarao tarao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephan-cr Great job! Thanks!

@tarao tarao merged commit ec135b5 into dimitri:master Jun 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants