-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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-14280: [Doc] R package Architectural Overview #14294
Conversation
|
Mind having a look at this PR @jonkeane @thisisnic? |
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.
Thanks for this, this is really great. I'm fine keeping it where it is, but it would be nice to link to it in our developers guide section somewhere too.
The only other thing that I might add is:
- When distributing a source package, we frequently bundle the libarrow source in a directory
tools/cpp
Lines 43 to 49 in 797f770
sync-cpp: cp ../NOTICE.txt inst/NOTICE.txt rsync --archive --delete --exclude 'apidoc' --exclude 'build' --exclude 'build-support/boost_*' --exclude 'examples' --exclude 'src/gandiva' --exclude 'src/jni' --exclude 'src/plasma' --exclude 'src/skyhook' --exclude 'submodules' --exclude '**/*_test.cc' ../cpp tools/ cp -p ../.env tools/dotenv cp -p ../NOTICE.txt tools/ cp -p ../LICENSE.txt tools/ sed -i"" -e "s/\.env/dotenv/g" tools/cpp/CMakeLists.txt - Do we want to mention anything about the
man
directory with documentation (and that it shouldn't be updated directly?) - Do we want to mention anything about the vignettes directory?
+1 @jonkeane, thank you! Will include suggested and ping when I am done 🙏 |
@jonkeane I think I have addressed all of you comments and the PR should be ready for another round of review. |
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.
Love this! Thank you so much
Co-authored-by: Jonathan Keane <jkeane@gmail.com>
@thisisnic would be great to have your final review also 🙏 |
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.
LGTM!
Benchmark runs are scheduled for baseline = 02c671a and contender = 6d815a4. 6d815a4 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
This PR adds an architectural overview of the R package to the New Contributor's Guide.
This work is co-authored by @thisisnic .