Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork268
Packer v2 Rewrite#1042
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Packer v2 Rewrite#1042
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| - Add tests where possible: testing a package manager can be a pain because of requirements to interact with the file system, etc. However, `packer`'s current tests do not cover much, and new code should at least strongly consider adding unit tests. | ||
| - Avoid giant functions: `packer`'s logic contains some monstrous functions (e.g., `manage`, the main compilation logic, etc.) that are difficult to work with. New code should strive to use more, simpler functions in a more modular design. | ||
| - Prioritize clean, performant code over total flexibility: `packer`'s design allows for a very flexible range of input formats. Although we don't want to completely lose this property, supporting all of these formats (and other use cases) has contributed to code bloat. If there's a choice to be made between losing some flexibility and significantly increasing the complexity of the code, seriously consider removing the complexity. | ||
| - Reduce redundancy: `packer` currently has an annoying amount of redundant/overlapping functionality (the worst offenders are the `requires`/`wants`/`after` family of keywords, but there's also duplication of logic in the utilities, etc.). The rewrite should aim to reduce this. |
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.
Generally this looks great!
Regarding redundancy, there's a lot of code-duplication inpacker.update andpacker.sync. Maybe there are reasons for this but if things are structured well, it seems likesync could just be something like:
packer.sync=function(..)packer.clean()packer.update(...)packer.compile()end
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.
That's the idea! I just haven't gotten to that part of the rewrite yet.
akinsho commentedSep 6, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@wbthomason thanks for opening it, looks good 👍🏿. I was thinking I'd raise some PRs against this to help move it on, but will probably start with smaller things like adding Emmy Lua annotations to as many places as possible. I personally heavily rely on those when writing Lua, so would be nice if the types in the code base were clearer. I've also been using the lazy loading pattern popularized by TJ inhttps://github.com/tjdevries/lazy.nvim and thought I might add it here? It defers the requiring of modules till they are actually indexed, so you can require all modules at the top of a file, but they won't actually be required till they are used. You can then combine this with locallazy=require('packer.lazy')--- @module'packer.config'localconfig=lazy.require('packer.config') Primarily because you've talked about reducing the cost of requiring packer, so this should help. |
Oh, brilliant! That looks quite useful. Separately, more annotations is also a great way to start - that's a stated goal of the rewrite and will make working with the codebase substantially nicer. Thanks! |
This is a WIP PR to complete the first stage ofthis
roadmap,
at least through the
loadfunction.Current progress:
The next steps are:
packermodule and your plugin specs.Contributions to this PR are welcome!