coderifous / jquery-localize

a jQuery plugin that makes it easy to internationalize your web site.
465 stars 142 forks source link

fixes #53 by removing sync xhr request #61

Closed ivoputzer closed 8 years ago

ivoputzer commented 9 years ago

in order not to break modern browsers

coderifous commented 9 years ago

This commit breaks the entire test suite. If you take the time to update the test suite as well, I can consider merging. Otherwise I will try to find the time for it.

ivoputzer commented 9 years ago

there must be something strange going on; all my tests are green and the ci status seems ok! though if we can troubleshoot the issue i will update the test suite accordingly.

could you please give it a fresh install and see what happens?

ivoputzer commented 9 years ago

@coderifous any updates on the subject?

coderifous commented 8 years ago

When the tests run, they include the built file under dist/, which are checked in for convenience:

https://github.com/coderifous/jquery-localize/blob/0ea27ed387f53c0cc4a2bb942c8bcd480d82f518/test/localize.html#L12

The test suite for this PR is passing because it's running against the unmodified code. If you run grunt then the dist/ files will be rebuilt, and the tests will run with your change. At which point virtually every test fails.

This is one of those instances where writing a failing test up front would've made the situation apparent more readily apparent. Maybe there's an easy change I can make to the CI setup to make that situation less likely to occur.