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

EJSON support #11

Closed
emgee3 opened this issue Mar 18, 2013 · 7 comments
Closed

EJSON support #11

emgee3 opened this issue Mar 18, 2013 · 7 comments

Comments

@emgee3
Copy link
Member

emgee3 commented Mar 18, 2013

I pushed a new branch that adds EJSON support. I included the EJSON smart package files in the ddp client as a temporary measure, until there's an EJSON npm package. I spoke with sixolet on IRC and it's on her list of things to do.

Feel free to check it out. It could use a bit more testing, but generally speaking it works.

@tmeasday
Copy link
Member

@emgee3 - do you need this urgently? If not, I say we should wait for @sixolet to upload EJSON somewhere before releasing this.

@emgee3
Copy link
Member Author

emgee3 commented Mar 18, 2013

No, I don't need it at all. Mostly just to have it around if someone else needs it.

@tmeasday
Copy link
Member

tmeasday commented Apr 3, 2013

@emgee3 - did you get any further with @sixolet here? I can ping her again about it if you'd like.

@emgee3
Copy link
Member Author

emgee3 commented Apr 18, 2013

@tmeasday @sarfata I updated the ddp-pre1-with-ejson branch now that we have the meteor-ejson npm module. So if you would take a look, that would be good.

It works with the built in EJSON types, though I don't know if we need to add anything else to support custom EJSON types.

@tmeasday
Copy link
Member

@emgee3 cool! good stuff.

I'm on vacation right now, but if either of you want to publish a new version of the package let me know your npm username and I'll give you access on npmjs.org

@emgee3
Copy link
Member Author

emgee3 commented May 28, 2013

Following up on this, I've filed this PR sixolet/ejson#2 which handles meteor/meteor#1001

Probably should add a few tests. The question I have is should we test the DDP client connecting to an actual Meteor server, or would mocking the components suffice? I don't actually write tests nearly as often as I should. Any input @sarfata or @tmeasday ?

@tmeasday
Copy link
Member

Hey @emgee3. You could take a look at Meteorite's atmosphere acceptance tests.

We use Mocha to connect to a local atmosphere running on port 3333 and inspect the results on the command-line. I'm not sure if they pass right now due to conflicts in the way that Meteorite + 0.6 Meteor work but it could give you some ideas for acceptance tests.

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

No branches or pull requests

2 participants