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

Update contribution guidelines documentation for kedro-datasets #1991

Merged
merged 6 commits into from
Nov 2, 2022

Conversation

ankatiyar
Copy link
Contributor

@ankatiyar ankatiyar commented Oct 31, 2022

Signed-off-by: Ankita Katiyar ankitakatiyar2401@gmail.com

Description

Development notes

Checklist

  • Read the contributing guidelines
  • 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 RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar ankatiyar requested review from jmholzer and noklam October 31, 2022 12:32
@noklam noklam requested review from stichbury and removed request for idanov October 31, 2022 12:54
Copy link
Contributor

@noklam noklam 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 making the changes. Can we also create/update this template in kedro-datasets? It's worth mentioning about the kedro-datasets since this is where most of the PRs coming from.

@@ -198,6 +198,8 @@ When you are ready to submit your code:
- [Kedro-Docker](https://github.com/kedro-org/kedro-plugins/tree/main/kedro-docker), a tool for packaging and shipping Kedro projects within containers
- [Kedro-Airflow](https://github.com/kedro-org/kedro-plugins/tree/main/kedro-airflow), a tool for converting your Kedro project into an Airflow project
- [Kedro-Viz](https://github.com/kedro-org/kedro-viz), a tool for visualising your Kedro pipelines
- [Kedro-Datasets](https://github.com/kedro-org/kedro-plugins/tree/main/kedro-datasets), a collection of all of Kedro's data connectors. These data
connectors are implementations of the `AbstractDataSet`
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest we put Kedro-Datasets at the top.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it to the top in b7b82b5

Copy link
Contributor

Choose a reason for hiding this comment

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

(Warning: purely pedantic) The order does look a little arbitrary - I'd usually vouch for alphabetical order but it's tougher here to decide priority, maybe datasets followed by airflow and then docker and viz?

docs/source/extend_kedro/custom_datasets.md Show resolved Hide resolved
@ankatiyar ankatiyar changed the title Update contribution guidelines to datasets Update contribution guidelines documentation for kedro-datasets Oct 31, 2022
ankatiyar and others added 2 commits October 31, 2022 15:25
Copy link
Contributor

@stichbury stichbury left a comment

Choose a reason for hiding this comment

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

LGTM 🏆

ankatiyar and others added 3 commits November 2, 2022 10:38
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
Signed-off-by: Ankita Katiyar <ankitakatiyar2401@gmail.com>
@ankatiyar ankatiyar merged commit a9817a9 into main Nov 2, 2022
@ankatiyar ankatiyar deleted the update-contribution-docs-kedro-datasets branch November 2, 2022 13:19
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.

Update contribution doc and PR template to make it clear to make dataset changes in kedro-datasets
4 participants