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

HubInvocation should probably take a JSONArray for arguments #22

Closed
robfe opened this issue Jun 17, 2013 · 4 comments
Closed

HubInvocation should probably take a JSONArray for arguments #22

robfe opened this issue Jun 17, 2013 · 4 comments

Comments

@robfe
Copy link

robfe commented Jun 17, 2013

In HubInvocation, there is the following code:

public String Serialize() {
    JSONArray arr = new JSONArray(mArgs);

This is fine for primitive parameters, but one my my hubs takes a complex object. Unfortunately, JSONArray's constructor isn't smart enough (like GSON) to serialise POJOs, so there is no way with the current API for my code to pass through some complex args. If HubProxy.Invoke simply took a JSONArray instead of a Collection<?>, then my code would have more control over what it can pass in. Other bits of the SignalA API deal with JSONArrays - so I think that would be a good approach.

Happy to submit a PR if you'd like.

P.S. awesome work :)

@erizet
Copy link
Owner

erizet commented Jun 17, 2013

Sounds like a good idea! Maybe it's possible to provide an overload for Invoke that take a JSONArray?!
Please submit a pull request, it will be the first in that case... :)

@robfe
Copy link
Author

robfe commented Jun 18, 2013

Well, i've made this code change at https://github.com/robfe/SignalA/commit/287874c89759181ac12f2cf0ab7bd5d07e0e4572.

However, I couldn't really do a nice clean pull request because a) It took a lot of tweaking to the project before I could get it compiling and b) i wanted to delete the metadata project from the repo but didn't actually get that working.

I too am a .NET dev so I'm not very good with eclipse & sorting out build paths etc.

I think that your project is awesome though! It would be really great it it was a repo you could just pull & run. That would make it much easier to submit PRs. Since I'm bad with java though, I don't know what it takes to get SignalA's build paths etc in a state where you don't have to make any changes to the source-controlled files before you can run it.

I do think though that it makes sense for the .metadata folder to be deleted from the repo, as there's all sorts of spurious changes going in there. See https://github.com/github/gitignore/blob/master/Global/Eclipse.gitignore

Thanks - Rob

@erizet
Copy link
Owner

erizet commented Jun 20, 2013

Two .Net guys lost in the big strange Java-world.... :)
I think that if I remove the .metadata folder and the .project files - you have to add each project to the workspace yourself after a pull. Perhaps it is preferable?

I miss the .sln files ;)

@erizet
Copy link
Owner

erizet commented Jun 27, 2013

"merged" in the new release. Thanks!

@erizet erizet closed this as completed Jun 27, 2013
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

No branches or pull requests

2 participants