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

Allow to use quelpa-use-package as source of truth for quelpa-cache #26

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kiennq
Copy link
Member

@kiennq kiennq commented Aug 3, 2022

This allows the quelpa-cache to be populated from the usage of quelpa-use-package only.
Since we're using quelpa-cache as a way to indicate which packages are managed by quelpa, this change allows those packages are correctly populated via the config files only.

Also using a simpler form instead of package-installed-p to check for package availability so that we don't need to load the whole package package when running from init file, improve Emacs init time

Copy link

@alphapapa alphapapa left a comment

Choose a reason for hiding this comment

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

I've left some comments on individual lines. Beyond those, please write a bit about the purpose of this PR and why it's necessary, to help me understand it.

quelpa-use-package.el Outdated Show resolved Hide resolved
quelpa-use-package.el Outdated Show resolved Hide resolved
quelpa-use-package.el Outdated Show resolved Hide resolved
(defvar quelpa-use-package-inhibit-loading-quelpa nil
(defgroup quelpa-use-package nil
"Quelpa handler for `use-package'."
:prefix "quelpa-use-package"

Choose a reason for hiding this comment

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

It's not harmful to specify the prefix, but it's unnecessary for options specified later in the same file.

:prefix "quelpa-use-package"
:group 'use-package)

(defcustom quelpa-use-package-inhibit-loading-quelpa nil
"If non-nil, `quelpa-use-package' will do its best to avoid

Choose a reason for hiding this comment

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

The first line of a docstring should be a complete sentence. If this one's is being updated, it might as well be fixed.

:type 'boolean
:group 'quelpa-use-package)

(defcustom quelpa-use-package-as-source-of-truth nil

Choose a reason for hiding this comment

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

The name of this variable seems unclear, and a bit mysterious. Could it be given a more descriptive one?

@kiennq
Copy link
Member Author

kiennq commented Aug 24, 2022

I've left some comments on individual lines. Beyond those, please write a bit about the purpose of this PR and why it's necessary, to help me understand it.

Thanks, let me clarify the purpose of this PR before addressing your comments.
Currently, quelpa is marking which packages it's managed via quelpa-cache, which is also used to store the custom recipes as well. The cache will be updated when we install a new package via quelpa, or get restored from the cache file if we specify the quelpa-persistent-cache-p.

So, if we don't install any new package with quelpa in this Emacs session and don't have quelpa-persistent-cache-p on, running quelpa-upgrade-all will do nothing (besides upgrading quelpa).
On other hand, if we have unnecessary packages in the quelpa-cache, doing upgrades will cost a lot of time since we're doing repo upgrades for all of those packages as well.

Since we have this :quelpa syntax with use-package, I think we can consider all packages that are declared with :quelpa in the config file as implicitly an entry in the quelpa-cache.
Using that and turn off quelpa-persistent-cache-p will make us have a clean quelpa-cache every time Emacs starts, with the exact information as specified in the config file.

@kiennq kiennq force-pushed the feat/sot branch 2 times, most recently from 5410bb6 to 5cd28b7 Compare November 2, 2024 03:53
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