Skip to content

Conversation

@thisisnic
Copy link
Member

@thisisnic thisisnic commented Oct 25, 2025

Rationale for this change

R package doc generation uses installed version of package so creates wrong docs if older version installed.

What changes are included in this PR?

Use devtools::load_all() to load the package first when running locally.

Are these changes tested?

CI should be happy, but I did test locally.

Are there any user-facing changes?

Nah.

@thisisnic thisisnic marked this pull request as ready for review October 25, 2025 17:11
@thisisnic thisisnic requested a review from jonkeane as a code owner October 25, 2025 17:11
Copy link
Member

@jonkeane jonkeane 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 this!

Comment on lines 135 to 138
warning(
"devtools is not installed. Using installed arrow package instead of current working code.\n",
"To generate accurate docs, install the current branch version of arrow first."
)
Copy link
Member

Choose a reason for hiding this comment

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

I don't mind leaving this as is, but I wonder if this actually should be an error? Is there a case where one would want to generate the stale docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair, I was just being conservative in my changes, but given this is a dev script I don't need to.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it's cool to do R CMD INSTALL . instead of devtools though? Just wanna give options.

@github-actions github-actions bot added awaiting merge Awaiting merge awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review awaiting merge Awaiting merge labels Nov 6, 2025
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Nov 6, 2025
@thisisnic thisisnic merged commit 4077efe into apache:main Nov 6, 2025
8 checks passed
@thisisnic thisisnic removed the awaiting change review Awaiting change review label Nov 6, 2025
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 0 benchmarking runs that have been run so far on merge-commit 4077efe.

None of the specified runs were found on the Conbench server.

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants