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

Add global minor mode (#16) #17

Merged
merged 1 commit into from
May 20, 2017
Merged

Add global minor mode (#16) #17

merged 1 commit into from
May 20, 2017

Conversation

DamienCassou
Copy link
Owner

No description provided.

@DamienCassou
Copy link
Owner Author

/cc @Fuco1: is this what you had in mind?

@DamienCassou
Copy link
Owner Author

I very much welcome code quality and best-practice feedback. Please don't hesitate.

@DamienCassou DamienCassou force-pushed the add-global-minor-mode branch 2 times, most recently from 71be2ca to e19013f Compare May 19, 2017 14:53
beginend.el Outdated
@@ -225,16 +225,41 @@ BEGIN-BODY and END-BODY are two `progn' expressions passed to respectively
(progn
(prodigy-last)))

(add-to-list 'beginend-modes (cons 'mu4e-view-mode-hook #'beginend-message-mode))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would move these values directly into the defvar default value. The reason is that if alter the code is somehow reordered Emacs might complain about undefined variables.

Also people seem to not like top-level forms which aren't defXYZ (I don't have a strong opinion about this though).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also format it like so:

(
 (...)
 (...)
 )

This looks weird but actually is "acceptable" in defvars and constants as it produces better diffs when lines are added/removed.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I would move these values directly into the defvar default value

done

Emacs might complain about undefined variables

The travis build would fail if that's the case as byte-compilation warnings are interpreted as errors in the Makefile (byte-compile-error-on-warn).

Also format it like so

This indeed looks weird but let's try it.

If you are fine with it, you can merge it :-).

@DamienCassou DamienCassou force-pushed the add-global-minor-mode branch from e19013f to 406c127 Compare May 20, 2017 05:17
@Fuco1 Fuco1 merged commit 75109cb into master May 20, 2017
@DamienCassou DamienCassou modified the milestone: v2.0.0 May 25, 2017
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