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

fix(classification): add data type to classification #91

Merged
merged 2 commits into from
Nov 2, 2022

Conversation

elsapet
Copy link
Contributor

@elsapet elsapet commented Nov 1, 2022

Description

Add data type to schema classification object

Also some refactoring / clean up of schema classification, namely:

  • Nest data type in classification / object patterns
  • Return DataTypable in classification object (instead of casting to a data type)
  • Pull out some private methods from schema classify method
  • Pull out some helper methods from schema classification file to a new classify_schema util
  • Change data type UUIDs to strings for simplicity (this is what we're using elsewhere in Curio for UUIDs)
  • Some refactoring of schema classification methods

Checklist

  • I've added test coverage that shows my fix or feature works as expected.
  • I've updated or added documentation if required.
  • I've included usage information in the description if CLI behavior was updated or added.
  • PR title follows Conventional Commits format

@swarmia
Copy link

swarmia bot commented Nov 1, 2022

@elsapet elsapet force-pushed the AMA-3088-add-data-type-uuid-to-schema-classification branch from 8bef317 to dc72d09 Compare November 1, 2022 09:09
@elsapet elsapet force-pushed the AMA-3088-add-data-type-uuid-to-schema-classification branch from dc72d09 to a613113 Compare November 1, 2022 09:37
refactor: clean up schema classify method
@elsapet elsapet requested a review from vjerci November 1, 2022 10:17
@elsapet elsapet merged commit 1d6247f into main Nov 2, 2022
@elsapet elsapet deleted the AMA-3088-add-data-type-uuid-to-schema-classification branch November 2, 2022 09:57
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.

None yet

2 participants