Skip to content

Conversation

@altkatz
Copy link

@altkatz altkatz commented Jun 3, 2014

tested on IE 8

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling a27c77a on altkatz:master into 0099463 on SheetJS:master.

@SheetJSDev
Copy link
Contributor

@altkatz thanks for pointing this out! It saddens me to see that jshint doesn't pick up on trailing commas

The gh-pages branch has a shim for IE8 and below which includes what is necessary. That probably should be included in master.

Can you make a few small changes:

  1. remove the trailing comma in the file bits/34_extprops.js -- https://github.com/SheetJS/js-xlsx/blob/master/bits/34_extprops.js#L14

  2. add the file shim.js (from the gh-pages branch) into the master branch (and update the README note to reflect that the shim.js is there)

  3. squash down to one commit.

I will make some more improvements in the next commit that will help prevent this. (I've been playing with https://www.npmjs.org/package/jscs locally and there are configuration options for detecting trailing commas)

@altkatz
Copy link
Author

altkatz commented Jun 3, 2014

@SheetJSDev shim.js added to master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 71dd522 on altkatz:master into 0099463 on SheetJS:master.

@SheetJSDev
Copy link
Contributor

@altkatz thanks for making the changes. If possible, can you squash the three commits down to one ?

@SheetJSDev
Copy link
Contributor

@altkatz I went ahead and squashed it. Even though the PR won't show as merged, the commit credits you a96b723

@SheetJSDev SheetJSDev closed this Jun 3, 2014
@SheetJSDev
Copy link
Contributor

Version 0.7.6 includes the referenced changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants