Open mcourtot opened 7 years ago
@mbaudis I noticed in your demonstration the usage of termLabel
and termId
today, when the schemas present term
and termId
.
It seems like to close this we might change term
to termLabel
? https://github.com/ga4gh/schemas/blob/master/src/main/proto/ga4gh/common.proto#L159
@david4096 I'm with a slight preference of termLabel (term_label?), but take whatever I get...
I'm always confused between the use of underscores or camel case; I find underscores easier to read - so term_label - and it would be more consistent with term_id in the current schema?
Yes, my apologies! I had been inspecting serialized JSON which uses camelcase, when the schemas themselves use underscores. So this issue will close with a change of term
to term_label
in the proto.
Thanks for clarifying @david4096! Is there a general rule that we use underscore instead of camel case in proto? Or are we keeping camel case for objects but underscores for attributes, such as in
message OntologyTerm {
string term_id = 1;
string term = 2;
}
Yep! https://developers.google.com/protocol-buffers/docs/style
Use CamelCase (with an initial capital) for message names – for example, SongServerRequest. Use underscore_separated_names for field names – for example, song_name.
So still term
, not term_label
in master.
@david4096 or @kozbo: Can you please put this into one of the next PRs against master?
message OntologyTerm {
string term_id = 1;
string term_label = 2;
}
(No idea why it is still term
).
Hi,
I thought we had discussed and agreed on term_id and term_label for the ontology term object at https://github.com/ga4gh/schemas/blob/master/src/main/proto/ga4gh/common.proto#L160
It seems Proto3 doesn't assign required or optional anymore - I was wondering if there was a way to specify, in response to #603, that the term_id was required whereas the term_label was optional. We should at least add a comment addressing the concern raised by @reece, namely that the ID is authoritative, the label is provided as convenience only, and it is recommended to refresh values based on IDs on a regular basis to deal with resources updates.