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

carryoverConcurrencyCount docs are vague #99

Closed
shellscape opened this issue Mar 9, 2020 · 7 comments
Closed

carryoverConcurrencyCount docs are vague #99

shellscape opened this issue Mar 9, 2020 · 7 comments

Comments

@shellscape
Copy link

Ahoy 🚢

The carryoverConcurrencyCount option could use some more verbose documentation. In toying with the package, it's not entirely clear what option value produce what behavior.

Perhaps even an examples directory that covered how this works?

@sindresorhus
Copy link
Owner

Can you be more specific about what's unclear about it? For now, I would recommend looking at its tests at that might clarify how it should be used.

// @Rafael09ED

@sindresorhus sindresorhus changed the title carryoverConcurrencyCount Docs are Vague carryoverConcurrencyCount docs are vague Mar 9, 2020
@Rafael09ED
Copy link
Contributor

Rafael09ED commented Mar 9, 2020

carryoverConcurrencyCount is asking whether it should carryover any incomplete promises into the next interval and apply that number against the limit cap. By effect, it determines whether the interval cap limits how many can start or how many can finish in the given time, if respectively set to false or true.
For example, if you can only have 5 connections at a time and are rate limited, you would set carryoverConcurrencyCount to true to make sure you don't go over 5 connections.
Does that help any?

@shellscape
Copy link
Author

@Rafael09ED that's the good stuff I was looking for in the readme. Thank you for adding that. Typically for options that are boolean, I recommend (and look for) a format such like:

If true, the thing behaves like one way. If false, thing behaves another way.

The paragraph you replied with above would be an absolute value-add to the readme!

@Rafael09ED
Copy link
Contributor

Rafael09ED commented Mar 10, 2020

Thank you for the good advice. How does this look for the new carryoverConcurrencyCount readme section?

carryoverConcurrencyCount

Type: boolean
Default: false

Whether any incomplete promises should be carried over into the new interval and applied against the intervalCap.
If true, intervalCap effectively limits how many can finish. If false, intervalCap limits how many can start.

Useful in preventing rate limit overages.

@shellscape
Copy link
Author

intervalCap effectively limits how many can finish

This threw me off, because it doesn't seem to relate (with that verbiage). We'd been talking about tasks/promises starting and this tosses finishing in the mix.

I'd tweak it a bit:

##### carryoverConcurrencyCount

Type: `boolean`\
Default: `false`

If `true`, specifies that an incomplete `Promise` should be carried over into the next interval and counted against the `intervalCap`. If `false`, an incomplete promise will continue to run independent of the next interval and will not count towards the `intervalCap`.

@sindresorhus
Copy link
Owner

Fixed by #100.

@shellscape
Copy link
Author

Thanks for tackling this.

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

No branches or pull requests

3 participants