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

API changes (BREAKING) #99

Open
geraintluff opened this issue Sep 25, 2013 · 3 comments
Open

API changes (BREAKING) #99

geraintluff opened this issue Sep 25, 2013 · 3 comments

Comments

@geraintluff
Copy link
Contributor

Implemented in separate branches, to avoid ruining everybody's code.

  • action(data, context, actionName, ...) - add data argument to match other methods
  • Remove data.indices()/data.index()/data.indexValue(), in favour of data.item() etc.
  • data.type() instead of data.basicType()
  • Remove schema.extendSchemas()
  • Anything called register???Listener(callback) probable replaced with on???(callback)
  • data.subPath() -> data.path() (with Relative JSON Pointer support)
  • data.get() and data.set() used to replace data.value(), and data.setValue()

It's worth doing a full review of the API, to spot anything else that is useless or inelegant.

@geraintluff
Copy link
Contributor Author

Opinions/suggestions are very welcome (@Ognian?). Any API calls that you found unintuitive, awkwardly-named, or that didn't exist and you think they should, please let me know.

Long-term, I might end up maintaining a backwards-compatability branch which keeps old method names / arguments, so that existing users don't have to re-write their code.

@Ognian
Copy link
Contributor

Ognian commented Sep 28, 2013

Hello,
the biggest problem I had and still have with the API is the way it is documented. Independent from that, if it is a good idea to document it via JSON itself or not, the place where the description has to happen, in my opinion, is inside the source code in a comment before the actual code. This would enable someone who reads the source code or debugs it to better understand it. I use (or better try to use, since I also don't like to document ...) JSDoc for this, since it is supported from the IDE I use (Webstorm). Maybe an option would be to generate the JSON for the API via the JSDoc comments…
But maybe just putting the JSON as it is now, in front of the procedure in a comment and extract it from there would be enough…

Regarding to the API itself, I think that your suggestions are really good.

When speaking of an major version and api update I would also suggest to think about making Jsonary an AMD module (https://github.com/amdjs/amdjs-api/wiki/AMD) which would allow it be be compiled by e.g. the closure compiler… (I'm using dojo build which gives an impressive speed gain when on e.g. tablets)

So, improving the modularization and documentation would make it much easier to keep the API evolving...

Ognian

@geraintluff
Copy link
Contributor Author

Nice - thanks for the suggestions. :)

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