Add tracker.getQueueName method#1217
Add tracker.getQueueName method#1217Peter Perlepes (igneel64) wants to merge 2 commits intomasterfrom
Conversation
Peter Perlepes (igneel64)
requested review from
Greg Leonard (greg-el),
Jethro Nederhof (jethron) and
Matus Tomlein (matus-tomlein)
BundleMonFiles added (6)
Total files change +95.43KB 0% Final result: ✅ View report in BundleMon website ➡️ |
Greg Leonard (greg-el)
left a comment
Greg Leonard (greg-el)
left a comment
There was a problem hiding this comment.
Looks good! Just a couple of thoughts I've had on the types.
I think I'm not properly following the types you've used here. I can see both:
'post' | 'get' | undefinedExclude<EventMethod, 'beacon'>
This seems to boil down to the same type, a potentially optional 'post' | 'get'. Could it be clearer to create a type alias for Exclude<EventMethod, 'beacon'>, which then could be used with ? as required?
| /** | ||
| * Returns the currently used queue name or if the `eventMethod` argument is provided, the queue name for the eventMethod. | ||
| */ | ||
| getQueueName: (eventMethod?: 'post' | 'get' | undefined) => string; |
There was a problem hiding this comment.
nit: This can be eventMethod?: 'post' | 'get' if you are using ?, right?
There was a problem hiding this comment.
This slipped in the PR, should be Exclude<EventMethod, 'beacon'> as in the implementation.
Jethro Nederhof (jethron)
left a comment
Jethro Nederhof (jethron)
left a comment
There was a problem hiding this comment.
LGTM.
What's the intended use case here vs just a "clearQueue" method or similar? Does this encourage people to mess with the queue themselves? Or is this for like a plugin API?
Peter Perlepes (igneel64)
force-pushed
the
feature/1215-expose-queue-name
branch
from
d27ef2d to
f286ece
Compare
3 participants
Add the
tracker.getQueueNamemethod.This method will return the currently used queue name or if provided arguments will return the one that would be used with the provided
eventMethod.close #1215