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

Create main developer guide for Python #11235

Merged
merged 15 commits into from
Aug 4, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jul 9, 2022

This PR adds a primary developer guide for Python. It provides a more complete and informative landing page for new developers. When #11217, #11199, and #11122 are merged, they will all be linked from this page to provide a complete set of developer documentation.

There is one main point of discussion that I would like reviewer comments on, and that is the section on directory and file organization. How do we want that aspect of cuDF to look?

@vyasr vyasr added 3 - Ready for Review Ready for review by team doc Documentation Python Affects Python cuDF API. non-breaking Non-breaking change labels Jul 9, 2022
@vyasr vyasr self-assigned this Jul 9, 2022
@github-actions github-actions bot removed the Python Affects Python cuDF API. label Jul 9, 2022
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Nice! Here are some drive-by comments (I got excited to see your work on this).

docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 9, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.10@d8c25a1). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.10   #11235   +/-   ##
===============================================
  Coverage                ?   86.47%           
===============================================
  Files                   ?      144           
  Lines                   ?    22856           
  Branches                ?        0           
===============================================
  Hits                    ?    19765           
  Misses                  ?     3091           
  Partials                ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

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

Some style comments/queries

docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
docs/cudf/source/developer_guide/developer_guide.md Outdated Show resolved Hide resolved
Copy link
Contributor

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

All my comments were addressed.

@vyasr vyasr requested a review from shwina July 18, 2022 20:46
Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@bdice bdice changed the base branch from branch-22.08 to branch-22.10 August 2, 2022 21:41
@vyasr
Copy link
Contributor Author

vyasr commented Aug 4, 2022

The status of the guide in this PR is good enough to merge as is with the current set of approvals. Future improvements to developer documentation can always be made without incurring undue additional overhead, so we're better off getting something merged sooner rather than later.

@vyasr
Copy link
Contributor Author

vyasr commented Aug 4, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit acadcf2 into rapidsai:branch-22.10 Aug 4, 2022
@vyasr vyasr deleted the docs/python_contributing branch August 4, 2022 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team doc Documentation non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants