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

Switch from camelCase to snake_case #127

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Switch from camelCase to snake_case #127

merged 1 commit into from
Feb 21, 2024

Conversation

vcschapp
Copy link
Collaborator

@vcschapp vcschapp commented Feb 20, 2024

Description

Brief description of the business purpose and effect of the pull request.

This commit switches properties and enumerators from camelCase naming to snake_case naming throughout the schema.

The purpose is to improve compatibility with SQL-based tools. Because SQL uses case-insensitive identifiers for historical reasons, SQL-based tools cause problems and information loss with camel-casing because they tend to treat all identifiers as if they were alllowercase or ALLCAPS.

In the Overture Schema Task Force meeting of 2024-02-07, the task force decided to adopt the snake_case proposal. The proposal originated in Jake's Schema Friction discussion document on the Overture Confluence.

While we believe that snake_case is slightly unconventional and, dare I (Vic) say also more unsightly in the JSON context, we feel that for SQL compatibility reasons, the pros of changing outweigh the cons.

Reference

List of relevant links to GitHub issues, PRs, and other documentation.

  1. https://github.com/OvertureMaps/schema-wg/issues/272
  2. https://wiki.overturemaps.org/x/SQGAAQ

Testing

Brief description of the testing done for this change showing why you are confident it works as expected and does not introduce regressions. Provide sample output data where appropriate.

  1. Updated examples and counterexamples and ensured all tests pass.
  2. In some cases improved the counterexamples by fixing longstanding bugs and adding the ext_expected_errors property to introduce some basic assertion sanity.
  3. Verified that the Docusaurus docs build and did a quick search through them for identifiers needing to be fixed.

Checklist

Checklist of tasks commonly-associated with schema pull requests. Please review the relevant checklists and ensure you do all the tasks that are required for the change you made.

  1. Add relevant examples.
  2. Add relevant counterexamples.
  3. Update in-schema documentation using plain English written in complete sentences, if an update is required.
  4. Update Docusaurus documentation, if an update is required.
  5. Review change with Overture technical writer to ensure any advanced documentation needs will be taken care of, unless the change is trivial and would not affect the documentation.

Documentation Website

Update the hyperlink below to put the pull request number in.

Docs preview for this PR.

@vcschapp vcschapp changed the base branch from main to dev February 20, 2024 21:46
@vcschapp vcschapp self-assigned this Feb 20, 2024
@vcschapp vcschapp force-pushed the snake_case branch 7 times, most recently from ea6274d to b558e86 Compare February 21, 2024 00:42
Copy link
Collaborator

@jenningsanderson jenningsanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Provided it passes validation and the docs build, let's get this merged down onto dev so we can continue building on top of it.

ibnt1
ibnt1 previously approved these changes Feb 21, 2024
@jwass jwass self-requested a review February 21, 2024 14:39
jwass
jwass previously approved these changes Feb 21, 2024
Copy link
Collaborator

@jwass jwass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like some class enum values in land and landuse still need to be converted.

@vcschapp vcschapp force-pushed the snake_case branch 2 times, most recently from 9c07a05 to 1e193c3 Compare February 21, 2024 16:02
This commit switches properties and enumerators from `camelCase` naming
to `snake_case` naming.

The purpose is to improve compatibility with SQL based tools. Because
SQL uses case-insensitive identifiers for historical reasons, SQL-based
tools cause problems and information loss with camel-casing because they
tend to treat all identifiers as if they were alllowercase or ALLCAPS.

In the Overture Schema Task Force meeting of 2024-02-07, the task force
decided to adopt the snake_case proposal:

OvertureMaps/schema-wg#272

The proposal originated in Jake's Schema Friction discussion document
on the Overture Confluence:

https://wiki.overturemaps.org/x/SQGAAQ

While we believe that snake_case is slightly unconventional, and dare I
(Vic) say also more unsightly in the JSON context, we feel that for SQL
compatibility reasons, the pros of changing outweigh the cons.
@vcschapp
Copy link
Collaborator Author

Looks like some class enum values in land and landuse still need to be converted.

Thanks for finding that. Fixed in my latest update.

@vcschapp vcschapp merged commit c153644 into dev Feb 21, 2024
2 checks passed
@vcschapp vcschapp deleted the snake_case branch February 21, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants