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

Replace startsWith for indexOf #144

Closed
wants to merge 1 commit into from
Closed

Conversation

styfle
Copy link

@styfle styfle commented Oct 25, 2018

It looks like PR #128 unintentionally broke compatibility with older versions of Node. So I changed it to a more compatible solution.

@styfle styfle mentioned this pull request Oct 25, 2018
12 tasks
@slackersoft
Copy link
Member

What version of Node.js are you seeing errors on? We run our tests against Node v4.x and above on Travis. My understanding from the node.js release schedule indicates that Node.js versions before that are no longer receiving updates, and folks are encouraged to update.

However, it does look like we didn't do a very good job in defining the supported engines in our package.json. If this is truly the only thing we need to change in Jasmine to support older versions of Node, it is probably worth it, but I'm concerned that supporting older versions could turn into a rabbit hole.

@styfle
Copy link
Author

styfle commented Oct 25, 2018

Yep, totally understand. We are at that point too with marked (see linked issue above).

I believe string.startsWith was introduced in Node v4.0.0 and our CI still runs v0.10 which was working fine until today 😃

I’ll continue discussion with marked team to see how soon we can deprecate 0.10 because right now, all builds are red ❌

@slackersoft
Copy link
Member

Closing this, since it looks like the issue has been resolved. Please let me know if this is incorrect.

@slackersoft slackersoft closed this Dec 4, 2018
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