Roadmap about classlist.js HOT 6 OPEN

@stevenvachon:

another preference is for single-purpose pull requests.

I agree ... about #53. IMO it does too much at once.

That's @beck responding to:

I fixed #48 in the "Consolidated closures" commit in #53.
I fixed whitespace in the "Whitespace fixes" commit in #53.
I rewrote the tests in the "Improve tests" commit in #53.

He's telling you he would like those to be separate PRs, not just separate commits. Condescension is not an appropriate response.

from classlist.js.

I fixed #48 in the "Consolidated closures" commit in #53.
I fixed whitespace in the "Whitespace fixes" commit in #53.
I rewrote the tests in the "Improve tests" commit in #53.

There are "..." buttons on those commits for you to read further commenting, in case you aren't familiar with GitHub.

prepublish is also ran on postinstall in npm <5. Likely not ideal at the moment. Use https://npmjs.com/in-publish

from classlist.js.

@stevenvachon another preference is for single-purpose pull requests.

I agree with @eligrey about #53. IMO it does too much at once. Before the closures can be consolidated, lets cleanup the whitespace. Before we touch every line with whitespace cleanup, lets get the tests in working condition. I would expect a fix for #48 to have a diff of ~2 lines, something like:

-}(self));
+}(typeof self !== 'undefined' ? self : this));

from classlist.js.

The whitespace commit occurs before the closure consolidation commit. You can click each commit to see its specific code changes.

from classlist.js.

-}(self));
+}(typeof self !== 'undefined' ? self : this));

This is what I do with every polyfill I deal with.

BTW I'm gonna send a PR regarding !a===!b at the bottom of the polyfill, tha last thing which is a thorn.

from classlist.js.

He's telling you he would like those to be separate PRs, not just separate commits. Condescension is not an appropriate response.

And I'm telling him and anyone else that they can go through the commits. I have little interest in redoing my PRs after all this time.

from classlist.js.