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

Cleanup some obsolete defadvice #417

Merged
merged 3 commits into from
Apr 7, 2024
Merged

Conversation

snogge
Copy link
Contributor

@snogge snogge commented Mar 5, 2024

No description provided.

@wasamasa
Copy link
Collaborator

wasamasa commented Mar 6, 2024

Please correct the formatting to not use tabs. Other than that, this looks like a logical change, given the bump to Emacs 25.1.

@Thaodan
Copy link
Contributor

Thaodan commented Mar 6, 2024

Please correct the formatting to not use tabs. Other than that, this looks like a logical change, given the bump to Emacs 25.1.

Does the rule to not use tabs apply to all Emacs Lisp files?

@wasamasa
Copy link
Collaborator

wasamasa commented Mar 6, 2024

Yes. To be fair, you'll be hard pressed to find any project besides Emacs itself still using them.

Resolves
circe.el:1944:2: Warning: ‘defadvice’ is an obsolete macro (as of 30.1); use ‘advice-add’ or ‘define-advice’
Resolves
lui-track.el: Warning: ‘defadvice’ is an obsolete macro (as of 30.1); use ‘advice-add’ or ‘define-advice’
lui-track.el: Warning: ‘defadvice’ is an obsolete macro (as of 30.1); use ‘advice-add’ or ‘define-advice’
@snogge
Copy link
Contributor Author

snogge commented Mar 10, 2024

Done. I also added a .dir-locals.el file with this setting.
And I see @Thaodan have also pushed a PR with this setting :)

@wasamasa wasamasa merged commit 5c67876 into emacs-circe:master Apr 7, 2024
8 checks passed
@wasamasa
Copy link
Collaborator

wasamasa commented Apr 7, 2024

Ironically, your patch did introduce more byte-compiler warnings due to the conditional definition of advice inside another function, so I've fixed that in a follow-up commit.

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.

3 participants