-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
ARROW-14753: [Doc] Steps in making your first PR - building C++ #11820
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some suggestions.
Failing build doesn't seem to be related (RStudio Package Manager error). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions below. Do you intend to fill the TODOs here or in another PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A very quick first pass!
Thanks everybody for the comments!
I wanted to do it later, but I think I gained some understanding to manage it now. In case the review will take longer as expected I would still think about putting it into a separate PR to make the process faster. As this is harder topic to write about for me (flags and env variables) it would be great to have some suggestions! |
Co-authored-by: Rok Mihevc <rok@mihevc.org>
Added the suggestions and the TODO parts (flags, env vars and CMake presets). I think this is ready for another round of review @thisisnic @jorisvandenbossche @pitrou Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few changes to suggest but otherwise looking good to me
Co-authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com> Co-authored-by: Nic Crane <thisisnic@gmail.com>
Benchmark runs are scheduled for baseline = 6896c6e and contender = 065b1fc. 065b1fc is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Add Building C++ and PyArrow sections of the New Contributor's guide.