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

Add js Date marshalling from rb Time #12

Merged
merged 6 commits into from
May 26, 2016
Merged

Conversation

seanmakesgames
Copy link
Contributor

No description provided.

@seanmakesgames
Copy link
Contributor Author

also, this is my first open-source contribution-- feedback is much appreciated!

@SamSaffron
Copy link
Collaborator

this looks great! it would be nice to support DateTime -> JS as well if possible cause it is fairly common for Rails apps, that said perfectly happy to merge this and great change!!!

👍

@seanmakesgames
Copy link
Contributor Author

Thanks @SamSaffron I can add that probably as well. Discussion needs to be had on that however, because it would be only one-way atm (there's only Date() on the JS side, so we'd be coming back as Time)

@SamSaffron
Copy link
Collaborator

I think one way is fine for this case, the timezone thing gets a bit hairy
so whatever we do we should document in the readme.

On Thu, May 26, 2016 at 11:41 AM, seanmakesgames [email protected]
wrote:

Thanks @SamSaffron https://github.com/SamSaffron I can add that
probably as well. Discussion needs to be had on that however, because it
would be only one-way atm (there's only Date() on the JS side, so we'd be
coming back as Time)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#12 (comment)

@SamSaffron SamSaffron merged commit 3865e94 into rubyjs:master May 26, 2016
@pan pan mentioned this pull request Mar 2, 2019
cataphract added a commit to cataphract/mini_racer that referenced this pull request May 6, 2020
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