ga4gh-beacon / specification

GA4GH Beacon specification.
Apache License 2.0
32 stars 25 forks source link

Markdown updates for 1.1.0 #279

Open teemukataja opened 5 years ago

teemukataja commented 5 years ago

As per request. Instead of copy-pasting from beacon.yaml I chose to update only the fields, and add new keys. Although, if you want consistency, I suggest writing examples to the former file and then copypasting the examples to the markdown file.

I strongly recommend abandoning double documentation, it is extra work to keep two separate documentation files in sync.

sdelatorrep commented 5 years ago

WDYT @juhtornr? Is there any reason to explain the fields, endpoints, parameters, etc, again in beacon.md? Because I agree with @teemukataja: it's annoying to maintain this file.

I suggest:

mbaudis commented 5 years ago

Markdown from YAML?! We generate the whole set of SchemaBlocks examples and .md ... from the YAML file through a simple Perl script. E.g.

leads to

Sorry in case I misunderstood the problem ...

sdelatorrep commented 5 years ago

This is very convenient for the .md files! (not so sure for the examples because we are currently showing real examples from a Beacon implementation, not theoretical examples) We should definitely explore implementing this in next release (v1.1.1 or v1.2).

mbaudis commented 5 years ago

@sdelatorrep Is there a planned transition to JSON Schema? Since we'll do the same for SchemaBlocks (documentation from schema using some "standard tooling" - ask @relequestal... - && doc for website as now.

juhtornr commented 5 years ago

WDYT @juhtornr? Is there any reason to explain the fields, endpoints, parameters, etc, again in beacon.md? Because I agree with @teemukataja: it's annoying to maintain this file.

I suggest:

  • removing the description of the endpoints, parameters and objects as this information is already in the YAML file.
  • keeping the rest of the document (e.g. design principles, security, errors, ...) as this is not in the YAML
  • keeping the examples of requests and responses

I think that it doesn't make sense to keep the same information in two different places. We should have just one place where the information is so therefore I agree with your proposal.

teemukataja commented 5 years ago

Hi @teemukataja, good work but I spotted several things to fix:

* the title still says v1.0.0

Title now says v1.1.0

  • In "Beacon object", the example for apiVersion still says v1.0.0 (not sure if it's necessary to update it as it's just an example, though). No longer applicable, as objects were removed.
  • in BeaconAlleleResponse the field beaconHandover is missing. Added.
  • in BeaconDatasetAlleleResponse there is a field beaconHandover but it should be datasetHandover. No longer applicable, as objects were removed.
  • the examples you pasted do not have these new fields beaconHandover or datasetHandover. Added missing fields.
  • definition of ADA-M object has not been updated. In the table where adamDataUse is listed, we should add a link to the repository as we do with consentCodeDataUse field, and remove the section "Adam Data Use object". No longer applicable, as objects were removed. WDYT @juhtornr? Is there any reason to explain the fields, endpoints, parameters, etc, again in beacon.md? Because I agree with @teemukataja: it's annoying to maintain this file.

I suggest:

* removing the description of the endpoints, parameters and objects as this information is already in the YAML file.

Removed

  • keeping the rest of the document (e.g. design principles, security, errors, ...) as this is not in the YAML Kept these.
  • keeping the examples of requests and responses Kept these. (Although Swagger displays responses as well, but not the requests. Would be nice to remove these as well and only keep example requests maybe?)