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 FDev IDs - two proposals #42

Merged
merged 4 commits into from
May 31, 2016
Merged

Add FDev IDs - two proposals #42

merged 4 commits into from
May 31, 2016

Conversation

Marginal
Copy link
Contributor

First proposal:

  • Add "systemId", "systemAddress" and "stationId" as optional fields to commodity/2, outfitting/1 and shipyard/1.
  • Add optional "id" to "commodities" array in commodity/2.
  • Add optional "id" to "modules" array in outfitting/1.
  • Add optional additional "shipIds" array to "message" in shipyard/1.

This is somewhat messy, and will break existing listeners that validate against the schemas until they update (because of the liberal use of "additionalProperties" : false in these schemas) which is rude.

Second proposal:

  • Create new commodity/3, outfitting/2 and shipyard/2 schemas, with new "system" and "station" structures to replace "systemName", "stationName".
  • Add optional "id" to "commodities" array in commodity/3 (as above).
  • Add optional "id" to "modules" array in outfitting/2 (as above).
  • Change "ships" in shipyard/2 to be an array of structures.

This produces IMO a cleaner and more logical structure. But existing listeners won;t understand messages in the new schema until they make some small code changes.
Example messages: commodity/3, outfitting/2, shipyard/2.
EDMC branch that prints messages in these proposed new schemas.

@Marginal
Copy link
Contributor Author

There's also a third potential option (not presented) of new schemas that just send IDs and no textual names. This could be a later step.

@AnthorNet AnthorNet merged commit afa3e08 into EDCD:master May 31, 2016
@AnthorNet
Copy link
Collaborator

OK, I see what the additionalProperties is doing!

@Marginal
Copy link
Contributor Author

Yes, I'm not sure that the changes to the existing schemas in my first proposal are wise.

Also, the IDs in this PR are incomplete. I'm now documenting IDs at https://github.com/EDCD/FDevIDs

Marginal added a commit to Marginal/EDDN that referenced this pull request Aug 2, 2016
This reverts commit afa3e08, reversing
changes made to cb35465.
@Marginal Marginal mentioned this pull request Aug 2, 2016
Marginal added a commit to Marginal/EDDN that referenced this pull request Sep 15, 2016
This reverts commit afa3e08, reversing
changes made to cb35465.
@Marginal Marginal deleted the IDs branch September 15, 2016 16:54
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.

3 participants