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

Type naming #38

Closed
sethvincent opened this issue Mar 9, 2023 · 2 comments
Closed

Type naming #38

sethvincent opened this issue Mar 9, 2023 · 2 comments
Assignees

Comments

@sethvincent
Copy link
Contributor

A couple of things to decide related to type naming:

  • should we include the schema version in the type name?
  • should we include the encoding in the type name? json vs protobuf

Some notes I had on this from the #32 pull request:

It might be ideal to have a naming convention like:

  • ObservationJson
  • ObservationProtobuf

Related: it looks like the protobuf comments export names like CoreOwnership_1 rather than CoreOwnership.

I have mixed feelings on whether the public types should include the version in the name. The main place that might matter is in migration code where it's needed to be explicit about versions and use multiple versions in the same code.

Maybe we could import latest version as just the name:

/**
* @property {import('mapeo-schema/types').ObservationJson} Observation
*/

/**
* @property {import('mapeo-schema/types').ObservationProtobuf} Observation
*/

As well as being more specific about the version needed:

/**
* @property {import('mapeo-schema/types').ObservationJson4} Observation
*/

/**
* @property {import('mapeo-schema/types').ObservationJson5} Observation
*/

A type without a version could be the latest version.

I can imagine there's a good argument for always including the version number and not having the convenience of importing the latest version by just the schema name.

@tomasciccola
Copy link
Contributor

protobuf names should be unique for a project, so that's the reason they have the version on the name. I'll probably rename the export so that the last version is the default one with the possibility of importing a specific version

@tomasciccola tomasciccola moved this from 🔖 Todo to 🏗 In progress in Mapeo - Sprint 2023 (Archived) Mar 21, 2023
@tomasciccola tomasciccola moved this from 🏗 In progress to 👀 In review in Mapeo - Sprint 2023 (Archived) Apr 5, 2023
@sethvincent
Copy link
Contributor Author

closed by #43

@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Mapeo - Sprint 2023 (Archived) Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

2 participants