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

Restructuring Contributing doc [skip ci] #9026

Merged
merged 30 commits into from
Aug 26, 2021

Conversation

iskode
Copy link
Contributor

@iskode iskode commented Aug 12, 2021

This PR is related to the raised issue here. It is a refactoring of the contributing doc with instructions to locally build cuDF in edit mode.

@GPUtester
Copy link
Collaborator

Can one of the admins verify this patch?

@iskode iskode changed the title contributing doc restructuring Restructuring Contributing doc Aug 12, 2021
@iskode
Copy link
Contributor Author

iskode commented Aug 12, 2021

This PR takes into account the change proposed here

@iskode iskode marked this pull request as draft August 12, 2021 12:52
Copy link
Contributor

@shwina shwina 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 your work on this @iskode! I love the new structure - it greatly improves the flow of the document! Left a few suggestions that I think could help with clarity.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@shwina shwina added doc Documentation non-breaking Non-breaking change labels Aug 12, 2021
iskode and others added 10 commits August 13, 2021 10:17
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
@iskode
Copy link
Contributor Author

iskode commented Aug 13, 2021

should we not add instructions for java developers?
Also should we not provide steps for kafka, custreamz edit mode installations ?

@shwina
Copy link
Contributor

shwina commented Aug 13, 2021

should we not add instructions for java developers?

That's a good question. @jlowe any thoughts here?

Also should we not provide steps for kafka, custreamz edit mode installations ?

That would be great! Personally, I don't have any experience with developing those libraries, so I'd ask @jdye64 to review once you've written up those instructions!

@jlowe
Copy link
Member

jlowe commented Aug 13, 2021

Feel free to lift instructions from the Java README or link to that document from here. If instructions are copied then we should probably have the Java README point back to the contributing doc to avoid duplicating instructions and allowing them to accidentally diverge over time.

@iskode
Copy link
Contributor Author

iskode commented Aug 13, 2021

I think linking Java README from here will be fine.
But, please @jlowe can you confirm that there are instructions for installing the java binding in edit mode as I don't know well maven ? Because these are main missing points for python cudf and dask_cudf that this PR attempts to cover. the current version only states how to build/install from source in a non editable way.
This makes me thinking about C++ libcudf, @shwina do instructions from this PR make it editable ?

@jlowe
Copy link
Member

jlowe commented Aug 13, 2021

installing the java binding in edit mode

Can you clarify what you mean by this? I think I understand what you mean for Python, e.g.: hacking directly on deployed source files from the install, but that doesn't have an equivalent in Java.

@iskode
Copy link
Contributor Author

iskode commented Aug 13, 2021

Ok @jlowe , thank you for recalling me. Yes java is a compiled language so edit mode makes only sense for python.

iskode and others added 6 commits August 26, 2021 10:11
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
iskode and others added 5 commits August 26, 2021 13:42
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
Co-authored-by: Ashwin Srinath <3190405+shwina@users.noreply.github.com>
@iskode
Copy link
Contributor Author

iskode commented Aug 26, 2021

I have incorporated suggested changes and refactor subsection titles to make them clear and short.
I believe we are good to go now except some omissions.

@shwina
Copy link
Contributor

shwina commented Aug 26, 2021

Thanks @iskode -- there's one last change I would ask for: we are inconsistent here about including the prompt ($) in the commands. Can we go ahead and omit the prompt in all the commands? It makes it easier for people to copy and paste commands into their terminal as well.

My apologies for the back and forth on this PR - really appreciate your patience and excellent improvements to this doc :)

@iskode
Copy link
Contributor Author

iskode commented Aug 26, 2021

Thanks @iskode -- there's one last change I would ask for: we are inconsistent here about including the prompt ($) in the commands. Can we go ahead and omit the prompt in all the commands? It makes it easier for people to copy and paste commands into their terminal as well.

You're right because even me I always copy and paste.

My apologies for the back and forth on this PR - really appreciate your patience and excellent improvements to this doc :)

No problem, I really enjoy the process of thinking/suggesting ideas back and forth.

@iskode
Copy link
Contributor Author

iskode commented Aug 26, 2021

Thank you @shwina for the suggestions. I've removed all prompt ( $ ) in the doc.

@shwina shwina marked this pull request as ready for review August 26, 2021 17:32
Copy link
Contributor

@shwina shwina left a comment

Choose a reason for hiding this comment

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

This LGTM. @robertmaynard could you please review/approve when you have a chance. Thanks!

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
iskode and others added 2 commits August 26, 2021 21:28
Co-authored-by: Robert Maynard <robertjmaynard@gmail.com>
Co-authored-by: Robert Maynard <robertjmaynard@gmail.com>
@raydouglass raydouglass changed the title Restructuring Contributing doc Restructuring Contributing doc [skip ci] Aug 26, 2021
@raydouglass
Copy link
Member

ok to test

@shwina
Copy link
Contributor

shwina commented Aug 26, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 86a4459 into rapidsai:branch-21.10 Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Documentation non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants