gatherer.request

[aeosynth]

sorting query strings to please your testing library is kinda insane imo. i would do one of these:

1. contort tests to please the code (saner than other way around)
2. patch nock to not care about the order of query string parameters
3. use a different testing library

i can squash these commits if you like

[davidchambers]

What are you doing in this neck of the woods, @Nami-Doc? :)

[aeosynth]

he likes stalking people

[vendethiel]

Do I? I warned you this would happen eventually, anyway

[davidchambers]

i would […] use a different testing library

I'm totally open to replacing Nock with something else, so feel free to make more drastic changes if you think it would make our lives easier.

[aeosynth]

i'm actually not a big tester, so don't really have an opinion, but sinon is pretty popular

http://sinonjs.org/ https://github.com/cjohansen/Sinon.JS

https://github.com/flatiron/nock/issues/82

You can use a path filtering regexp or function that will be called on every request by using .filteringPath(): https://github.com/flatiron/nock#path-filtering

manually filtering paths is still pretty stupid, but it would be easy enough

[aeosynth]

moved sorting shenanigans and querystring dependency into test

[aeosynth]

would be a bit better if details were passed w/o munging

[davidchambers]

Can you provide a 50-character synopsis of these changes? I'd find this helpful for two reasons:

• I'd like to understand your high-level objective, and
• I'd like to know what your commit message should say when I come to squash these commits before pushing them to master. ;)

[aeosynth]

wrap request; refactor; fix #45

[aeosynth]

the ease of writing a request did not actually improve much:

url = gatherer.card.url 'Details.aspx', details; request url, cb

->

query = gatherer.card.query details; gatherer.request 'Pages/Card/Details.aspx', query, cb

but this pr does shave 57 lines, and makes the internals nicer imo

[aeosynth]

what exactly is the point of src/request.coffee? optimizing for browserify?

whoops, accidentally closed

[davidchambers]

It's for Browserify, yes. My thought was that when we "browserify" Tutor, we'll define a function which wraps jQuery.ajax to make it compatible with Request's interface.

[aeosynth]

you could also use something like superagent which natively works in both client and server. but please don't make optimizations prematurely

[vendethiel]

Waw for commonjs-everywhere :)

[aeosynth]

Do you like that better than brunch now?

[vendethiel]

That's definitely not for the same purposes. Brunch offers html, js and css compiling to create a single-page app, you can use amd, commonjs, your wrapper or nothing. (i'm using it in my angular-test project, because I want a web page) Whereas commonjs-everywhere is meant to provide a common "env" (both browser and node) by replicating native modules, allowing use of /node_modules/, etc (brunch only uses node modules for its plugins). I use for my userscript because all I want is a single .js

[aeosynth]

What if I want js html CSS compiled into a single bookmarket?

[vendethiel]

It's like an userscript, so the latter.

[davidchambers]

I spent a couple of hours refining these changes to their very essence, à la Jony Ive. :)

If you're happy with 0e2f6409df58e771069fc7c1fa0e0f985a8a57d3, @aeosynth, I'll pull it into master and close this pull request. In future, please open a separate pull request for stylistic changes.

[aeosynth]

lgtm