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

Use cider--font-lock-ensure for compatibility with Emacs 24.5 #2057

Merged
merged 1 commit into from
Jul 22, 2017

Conversation

xiongtx
Copy link
Member

@xiongtx xiongtx commented Jul 20, 2017

Fixes #2056.

@bbatsov
Copy link
Member

bbatsov commented Jul 20, 2017

This should be in the changelog as it's a bugfix for a stable release.

We should probably update travis to run the byte-compilation checks for all supported Emacs version (https://github.com/clojure-emacs/cider/blob/master/.travis.yml#L8) to avoid such issues down the road.

@xiongtx
Copy link
Member Author

xiongtx commented Jul 20, 2017

Added changelog entry and full test matrix for Emacs versions and tests.

At some point we should probably drop support for Emacs 24.4 and just go w/ 24.5.

I'm not familiar enough w/ Travis to know whether this line under matrix is necessary.

@xiongtx
Copy link
Member Author

xiongtx commented Jul 20, 2017

Looks like filling out the test matrix is already paying off--just discovered we're using directory-files-recursively, which is introduced by Emacs 25, without defining it in cider-compat.

@bbatsov
Copy link
Member

bbatsov commented Jul 21, 2017

I see that the tests are still failing because of directory-files-recursively. I'm also not sure is the checkdoc task working properly. I remember a few PRs with checkdoc errors that weren't flagged for some reason. Also - checkdoc hasn't changed much in a long time, do we need to run it on every supported Emacs?

@xiongtx
Copy link
Member Author

xiongtx commented Jul 21, 2017

I'm mystified as to why directory-files-recursively is causing the test to fail. I've posted a question on Emacs StackExchange.

I'm also not sure is the checkdoc task working properly. I remember a few PRs with checkdoc errors that weren't flagged for some reason.

checkdoc produces warnings, not errors, so the tests still pass.

There's no option to make checkdoc use error instead of warn, but we can run checkdoc-file on all source files and check whether the *Warnings* buffer exists after that. If it does, we can exit w/ an error.

For some reason checkdoc is not reporting warnings for Emacs 24.x, even though I've copied in check-doc error from Emacs 25 into cider-checks.el. Will look into this further when I have time.

checkdoc hasn't changed much in a long time, do we need to run it on every supported Emacs?

I don't see the harm. Travis CI has a 200 build limit IIRC and we're not anywhere near that.

@xiongtx xiongtx force-pushed the font-lock-ensure branch 4 times, most recently from 48eb8c1 to 0933f28 Compare July 22, 2017 04:30
@xiongtx
Copy link
Member Author

xiongtx commented Jul 22, 2017

I'm going to open another PR w/ the TravisCI stuff. We'll keep this PR focused on font lock.

@bbatsov bbatsov merged commit 9773544 into clojure-emacs:master Jul 22, 2017
@xiongtx xiongtx deleted the font-lock-ensure branch July 22, 2017 04:37
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