-
Notifications
You must be signed in to change notification settings - Fork 405
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
Feat transform commands #958
Conversation
commands/addJob-9.lua
Outdated
@@ -56,7 +56,7 @@ local parentQueueKey | |||
local parentData | |||
|
|||
-- Includes | |||
--- @include "includes/destructureJobKey" | |||
--- @include "destructureJobKey" |
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 it is good to keep the underlying structure, it is a pity to only support a global flat directory structure.
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.
From my experience, a flat structure can get unruly. My includes
folder currently has 50 files, and I'd imagine that over time "includes" could greatly outnumber main scripts in bullmq
.
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.
Another thought: I was planning on refactoring some of the scripts to be more reusable - for example removeJob-1.lua
(to implement removeJobsByPattern
). I think it would be confusing to have a main script and its utility file named similarly in the same folder.
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.
Sounds good to me, I'll try to transform our scripts keeping the folder structure in some way
"ioredis": "^4.28.2", | ||
"lodash": "^4.17.21", | ||
"minimatch": "^3.0.4", |
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.
What does minimatch provide that cannot be achieved with glob?
The thing with glob vs minimatch is that glob seems to be much better mantained and used.
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.
minimatch is used in glob https://github.com/isaacs/node-glob/blob/master/package.json#L23 and both libraries belongs to the same maintainer too
ref #901 #341 #469