-
Notifications
You must be signed in to change notification settings - Fork 114
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
[craftedv2beta] Module documentation: crafted-updates
#340
[craftedv2beta] Module documentation: crafted-updates
#340
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few things to discuss (nothing serious), otherwise looks good.
modules/crafted-updates-config.el
Outdated
|
||
;; UPDATE: [2023-02-09 Thu] `crafted-emacs-home' is now set on load, | ||
;; this can be updated to use that value for checking on updates from | ||
;; This uses `crafted-emacs-home', which is set on load by if it hasn't been |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be reading this wrong, but is there a "by" too much?
This uses `crafted-emacs-home', which is set on load if it hasn't been
set by the user or `crafted-init-config'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, that's right. Good eye, thank you!
docs/crafted-updates.org
Outdated
|
||
You can use the functionality provided by this module in three ways: | ||
|
||
1. On startup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure on this mini "table of contents" when the content is directly following.
Especially in info-buffer, it looks a bit funny given 4-star headings are turned into numbered entries:
1. On startup
2. Regularly
3. Manually
1. On Startup
...
I'd personally remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I agree.
Would you just remove it or turn it into a regular introductory sentence? "You can use the functionality provided by this module in three ways: on startup, regulary and manually."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think putting it into the introductory sentence makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think putting it into the introductory sentence makes sense.
Think so, too. Amended it. Thanks for the feedback!
Regenerating info buffer, otherwise consider this approved from my end |
Wow, somehow I missed this, sorry for the delay. |
Next in line from #128.