SignalK / specification

Signal K is a JSON-based format for storing and sharing marine data from different sources (e.g. nmea 0183, 2000, seatalk, etc)
Other
91 stars 69 forks source link

[WIP] Separate Javascript schema API from Schema Specification #435

Open bkp7 opened 6 years ago

bkp7 commented 6 years ago

PR for issue #389.

The api has been separated out and published on npm @bkp7/schema-js-api for now.

This PR references version 0.0.5 of @bkp7/schema-js-api which is basically the code cut out of this repository as is.

Subsequently, I have added some tests, fixed some issues and started on some documentation for the API which is currently at 0.0.7. There is a need, having separated out the API, to be able to load different versions of the schema as required, including local. As a result there is a small change to the API with .schemas being replaced with .getSchemas(). With this small change applied I am ensuring that later versions of the API work as expected with specification.

The API is currently light on documentation, as it never had to stand alone before. Before spending too much time tidying up the API I'd appreciate knowing that this will be adopted. There is no particular reason that the split shouldn't happen on the code as is, since the API is no less documented than it was previously.

tkurki commented 6 years ago

I would have approached this myself by extracting the js api first within the same repository, but no matter.

So how do things work now, when this repo's tests need the js api and the js api should use the schemas in this repo? I looked at the code briefly - it loads the published schema, right?

How about restructuring the js api so that it has a separate entrypoint for validation/schema related things? Something like require('@signalk/api').schemaApi(require('@signalk/signalk-schema')).

bkp7 commented 6 years ago

Currently the separated api uses a local copy of the schema if it's available, otherwise http://signalk.org/specification/1.0.0/schemas/signalk.json. This allows both sets of testing, but yes, it needs to be changed!

Seems to me we need to be able to load:

As regards published versions can we expose a list somewhere? A directory listing at signalk.org/specification/ ? I know that's where the playground is at the moment but it would be good if it could be self documenting. Maybe FTP?

Also, it would be useful to have a list of top level schemas automatically available for each version. Is the schemas folder structured the way it is for a reason, or would it be better to only have top level schemas in that folder? That way it would also be self documenting and would enable a list of schemas to be read. This would be useful for drop downs etc.

Agreed re separate entry point. My plan was to separate the api, document it, and then look at its structure and plan a way forward. I was reluctant to change much as I wasn't sure how many projects use the api. I'm currently aware of:

fabdrol commented 6 years ago

@bkp7 what's the beat on this PR?

bkp7 commented 6 years ago

@fabdrol, we need to establish whether there are any other sets of code using the API which will need to be modified to use the separated API. If you are aware of any...

It's my next thing to look at to deal with the conflicts as it would be good to get it separated from the spec. prior to doing the schema cleanup.

fabdrol commented 6 years ago

@bkp7 I've been playing with some modules that show dependants.. different output and may not even be complete, due to the fact that some check the current version, and one checks every other version except current.. anyway, I've managed to find these so far (in addition to yours, may be some overlap)

signalk-aishub-ws, signalk-pebble-mydata, n2k-signalk, signalk-server, n2k-signalk, signalk-testclient, kgauge, signalk-polar-graphing

bkp7 commented 6 years ago

@fabdrol, I've rebased to master so this should be good to implement.

The first thing we need to do is get the github and npm repository for the api code set up/transferred to be part of the SignalK project. Once that's done we can put a PR into each dependent project to point at the new api code. As stated above 0.0.5 is designed to be a simple reference replacement for what is present now. Perhaps easiest to contact me direct to implement transfer.

fabdrol commented 6 years ago

@bkp7 thanks, let's get a few more opinions before we move on with this one though. It's a consequential change, and we should probably do the changes to the dependant projects around the same time (at least the parsers, @signalk projects, etc)

@SignalK/core input?

tkurki commented 6 years ago

How about

  1. create the new api in a new repo
  2. change specification to use the new api (part of this PR, but from @signalk/schema-js-api)
  3. change the dependents
  4. deprecate & remove the old one (part of this PR)

These do not need to happen in lockstep or synchronously. The only danger is to leave some of the steps dangling.

bkp7 commented 6 years ago

I've found 91 files in github (search: signalk/signalk-schema language:JavaScript) across an unknown number of repositories which reference @signalk/signalk-schema.

Most of them I've looked at (about 8) use a fixed version reference eg. "1.0.3". Some use the caret eg. "^1.0.3".

I believe that this PR needs to push the specification version number to 2.0.0 and that way each dependent app can change their refs as/when they see fit (we should provide documentation as to what's required).

Therefore I propose:

  1. create the new api in a new repo
  2. note the api in the 1.0.x documentation as being deprecated and advise to use the new api repo.
  3. apply this PR to the schema cleanup branch which will also require a major version bump anyway and because one of the objectives of cleanup is to lose the TV4 dependency which will require changes to the api.
fabdrol commented 6 years ago

@bkp7 I can merge this into schema-cleanup if you think it's ready for that, that'll just leave the other to things todo.

bkp7 commented 6 years ago

@fabdrol, no please don't merge yet - it will need modifying to point to the new location. What I'm proposing is that we separate the api and use it in the master whilst leaving the deprecated api files in place within the master (suitably annotated) so that other projects won't break. That would just leave deleting the deprecated files to be done in schema-cleanup.

That being the case, could you setup/move @bkp7/schema-js-api to @signalk/schema-js-api and get the Travis, coveralls and Scrutinizer accounts linked up, or give me permissions/access to do that?

fabdrol commented 5 years ago

@bkp7 last year you mentioned that you didn't want this merged yet as there was still work to do. Any progress? Or should we close due tot lack of activity, make a new issue for it and look at it again later?