search

MoOx / pjax

Easily enable fast Ajax navigation on any website (using pushState + xhr)
MIT License
1.46k stars 125 forks source link

Fix bugs and add tests #145

Closed BehindTheMath closed 6 years ago

BehindTheMath commented 6 years ago

This PR has a bunch of various bug fixes for the code and tests, and many new tests. It's based on #144, so that needs to be merged first.

@robinnorth I cherry-picked your commit from the fix/missing-tests branch (Add tests for lib/util/noop.js - 5e7341d).

robinnorth commented 6 years ago

@BehindTheMath nice one, that's a substantial PR! I'll take a proper look at it as soon as I have time.

BehindTheMath commented 6 years ago

Thanks. I didn't originally plan on it being so big, but I was aiming for as close to 100% coverage as I could, and I kept adding more tests and finding more issues.

If it's too big, I can break it up into smaller PRs.

BehindTheMath commented 6 years ago

@robinnorth Do you think you'll have time to look this over, or should I just merge it? I would love to have a second pair of eyes on it, but I know you said you've been busy lately.

robinnorth commented 6 years ago

Sorry, I've been very tardy in reviewing this for you. I have indeed been busy, but I'll make some time to check it out later and get back to you.

BehindTheMath commented 6 years ago

Thank you, I appreciate it.

BehindTheMath commented 6 years ago

I deleted the fix/missing-tests branch, since I cherry-picked the only commit from there into this PR, and we're close to 100% coverage.

robinnorth commented 6 years ago

Cool, thanks!