Add maxWorkers option

[anandthakker]

Also sets maxWorkers on the debug page to 3, on the theory that a pretty
large majority of users have 4 cpus.

Closes #2022

[anandthakker]

cc @lucaswoj

I don't see an especially good way to add meaningful tests for this, since it's basically about passing a number through to new Style() and then to new Dispatcher(). I've used something like proxyquire in other projects to mock CJS module dependencies, but not sure that that's something yall want to adpot

[lucaswoj]

I don't see an especially good way to add meaningful tests for this

Looks like you made one test start failing. That might be a good place to start looking. 😉

[lucaswoj]

Why maxWorkers? Isn't this literally the number of workers?

[mourner]

👍 numWorkers

[anandthakker]

@lucaswoj @mourner I considered "num", but chose "max" -- this was the argument in my head:

maxWorkers is really what the user (me) wants to control, whereas numWorkers is more of an internal property. The fact that they are the same right now is an implementation detail; I could imagine the implementation changing to, say, construct/destroy workers on demand (especially given the possibility of worker pooling).

That said, I don't feel especially strongly about this, so just lemme know what yall think

[lucaswoj]

Right now, this property literally controls the number of spawned workers. If we change the implementation in the future, we can change the property name 😄

Let's go with workerCount for an abbreviation-free, grammatically correct, and best-practice compliant name.

[lucaswoj]

Happy to 🚢 this with a test case, and agreenment on the parameter name.

[anandthakker]

Looks like you made one test start failing. That might be a good place to start looking. 😉

Oh dear. 😊

Thanks, will do.

[anandthakker]

@lucaswoj Fixed the failing test and added another, but the build still fails because coveralls (I think) doesn't like my fork:

/home/ubuntu/mapbox-gl-js/node_modules/coveralls/bin/coveralls.js:18
        throw err;
        ^
Bad response: 422 {"message":"Couldn't find a repository matching this job.","error":true}

[lucaswoj]

The coveralls error should have been (might have been?) fixed by 1b25043. Have you rebased in the last day?

[anandthakker]

The coveralls error should have been (might have been?) fixed by 1b25043. Have you rebased in the last day?

I had not! Just rebased and force pushed.

[lucaswoj]

Rad! Looks like it is working now.

[anandthakker]

Sweet!

Did you see my note about the option name? Easy to change, just lemme know.

[anandthakker]

Amended the commit:

• s/maxWorkers/workerCount/
• Add jsdoc for workerCount option

I believe this should now be ready

[anandthakker]

➡️ #2666