Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Remove trailing commas for IE 9 compatibility" #622

Merged
merged 2 commits into from
Jan 11, 2017

Conversation

gagern
Copy link
Collaborator

@gagern gagern commented Jan 11, 2017

This reverts commit 4d2e46e.

Having trailing commans makes diffs easier to read as it avoids modifying a line just to add a trailing comma if there is another item to add at the end of a list. There are plans to switch to ES6 notation and to translate that to ES5 as part of the build process (#617). Since that translation would remove trailing commas, the IE9 problems that originally motivated the commit should vanish soon.

This reverts commit 4d2e46e.

Having trailing commans makes diffs easier to read as it avoids modifying a
line just to add a trailing comma if there is another item to add at the end
of a list.  There are plans to switch to ES6 notation and to translate that
to ES5 as part of the build process.  Since that translation would remove
trailing commas, the IE9 problems that originally motivated the commit
should vanish soon.
This makes eslint happy again.
Copy link
Contributor

@xymostech xymostech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gagern
Copy link
Collaborator Author

gagern commented Jan 11, 2017

@xymostech are you waiting for some other input before merging? Do you want me to merge this myself?

@xymostech
Copy link
Contributor

Nope, I was just waiting for you to merge it! (same goes for #623 and #615) I think in general I'll mention if I think somebody else should be looking. :)

@gagern gagern merged commit 59b8753 into KaTeX:master Jan 11, 2017
@gagern gagern mentioned this pull request Jan 12, 2017
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.

2 participants