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

Fix issue 257 #310

Merged
merged 3 commits into from
Nov 1, 2016
Merged

Fix issue 257 #310

merged 3 commits into from
Nov 1, 2016

Conversation

philippsimon
Copy link
Contributor

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Description

@philippsimon
Copy link
Contributor Author

philippsimon commented Oct 23, 2016

@kvz could you please rerun the Test for node.js 0.12?
I tested it on my computer and this weird error in Travis didn't happen there.

@kvz
Copy link
Collaborator

kvz commented Oct 24, 2016

Thanks for the PR! Sure, running here https://travis-ci.org/kvz/locutus/builds/169943784

@philippsimon
Copy link
Contributor Author

ok - strange: It's still happening. I take a look at it tonight.

@geriux
Copy link

geriux commented Oct 30, 2016

So thankful for this PR hope it gets merged soon, it fixes some issues with spanish characters like áéíóú, etc. 👍

@philippsimon
Copy link
Contributor Author

Sorry for not fixing it before - had a busy time.
I got it fixed by using function _unserialize (data, offset) { instead of var _unserialize = function (data, offset) {

@kvz is there a special reason, why in the project var functionName = function(...) is always used instead of the more common form function functionName (...)?

@kvz
Copy link
Collaborator

kvz commented Nov 1, 2016

@philippsimon I think that's just some poor taste from 2007-me tbh.

Thanks for the fixes! And especially adding the tests & attribution! ❤️ Merging this.

@kvz kvz merged commit 61c2211 into locutusjs:master Nov 1, 2016
@philippsimon
Copy link
Contributor Author

@kvz hahaha - ok ;) and you're very welcome for that fix. I'm really happy that you started this project!

@geriux
Copy link

geriux commented Nov 4, 2016

@kvz Hi! Any plans to release this PR any time soon? 🙏

@kvz
Copy link
Collaborator

kvz commented Nov 8, 2016

2.0.6 was just released!

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