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

chore: Move kedro-catalog JSON schema to kedro-datasets #952

Merged
merged 9 commits into from
Dec 12, 2024

Conversation

chrisschopp
Copy link
Contributor

Description

Resolving 4258

Development notes

Moved static/jsonschema/ from kedro (there is a corresponding PR there) to kedro-datasets.

Checklist

  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the relevant RELEASE.md file
  • Added tests to cover my changes
  • Received approvals from at least half of the TSC (required for adding a new, non-experimental dataset)

Signed-off-by: Chris Schopp <56572144+chrisschopp@users.noreply.github.com>
Signed-off-by: Chris Schopp <56572144+chrisschopp@users.noreply.github.com>
@chrisschopp chrisschopp changed the title Resolving #4258 Move kedro-catalog JSON schema to kedro-datasets Dec 1, 2024
@chrisschopp chrisschopp changed the title Move kedro-catalog JSON schema to kedro-datasets chore: Move kedro-catalog JSON schema to kedro-datasets Dec 1, 2024
Signed-off-by: Chris Schopp <56572144+chrisschopp@users.noreply.github.com>
Signed-off-by: Chris Schopp <56572144+chrisschopp@users.noreply.github.com>
@chrisschopp chrisschopp marked this pull request as ready for review December 10, 2024 02:38
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @chrisschopp! This mostly looks good, but now some of the datasets are still part of the core Kedro package, so I don't think those should now still be in the list here.

kedro-datasets/static/jsonschema/kedro-catalog-0.19.json Outdated Show resolved Hide resolved
kedro-datasets/static/jsonschema/kedro-catalog-0.19.json Outdated Show resolved Hide resolved
kedro-datasets/static/jsonschema/kedro-catalog-0.19.json Outdated Show resolved Hide resolved
…nce they remain in Kedro core

Signed-off-by: Chris Schopp <56572144+chrisschopp@users.noreply.github.com>
Copy link
Member

@merelcht merelcht left a comment

Choose a reason for hiding this comment

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

Thanks @chrisschopp ! LGTM 👍

Can you add the change to the release notes as well? Together with your name in the list of contributors: https://github.com/kedro-org/kedro-plugins/blob/main/kedro-datasets/RELEASE.md#community-contributions

@merelcht merelcht requested a review from ankatiyar December 11, 2024 09:35
Copy link
Contributor

@ankatiyar ankatiyar left a comment

Choose a reason for hiding this comment

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

Thanks @chrisschopp!

Signed-off-by: Chris Schopp <56572144+chrisschopp@users.noreply.github.com>
@merelcht
Copy link
Member

Thanks a lot for the contribution @chrisschopp ! I'll merge this now 🎉

@merelcht merelcht merged commit 2df011c into kedro-org:main Dec 12, 2024
47 checks passed
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.

3 participants