FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-43623: [R] remove libarrow backwards compatibility enforcement by jonkeane · Pull Request #46491 · apache/arrow · GitHub

/ arrow Public

GH-43623: [R] remove libarrow backwards compatibility enforcement - #46491

Merged
jonkeane merged 2 commits into
apache:mainfrom
jonkeane:43623_remove_compatibility
May 26, 2025
Merged

GH-43623: [R] remove libarrow backwards compatibility enforcement#46491
jonkeane merged 2 commits into
apache:mainfrom
jonkeane:43623_remove_compatibility

Conversation

jonkeane commented May 18, 2025
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Rationale for this change

Remove checks and enforcement of minimum versions. See discussion in the issue. This caused more confusion and discussion than it was helpful. We have the CI in git history if we need it.

What changes are included in this PR?

Removing the CI job

Are these changes tested?

They are the tests

Are there any user-facing changes?

No, we are still technically compatible with 19.0.1, but we don't explicitly support that or use it anywhere. The R package version should match the libarrow version to ensure compatibility — using anything else might work but is off label.

Copy link
Copy Markdown

⚠️ GitHub issue #43623 has been automatically assigned in GitHub to PR creator.

github-actions Bot added the awaiting committer review Awaiting committer review label May 18, 2025

Copy link
Copy Markdown
Member Author

@github-actions crossbow submit -g r

Copy link
Copy Markdown

Revision: bf18353

Submitted crossbow builds: ursacomputing/crossbow @ actions-a14f05c971

Task Status
r-binary-packages
r-recheck-most
test-r-arrow-backwards-compatibility
test-r-depsource-bundled
test-r-depsource-system
test-r-dev-duckdb
test-r-devdocs
test-r-extra-packages
test-r-gcc-11
test-r-gcc-12
test-r-install-local
test-r-install-local-minsizerel
test-r-linux-as-cran
test-r-linux-rchk
test-r-linux-sanitizers
test-r-linux-valgrind
test-r-m1-san
test-r-macos-as-cran
test-r-minimal-build
test-r-offline-maximal
test-r-offline-minimal
test-r-rhub-debian-gcc-devel-lto-latest
test-r-rhub-debian-gcc-release-custom-ccache
test-r-rhub-ubuntu-release-latest
test-r-rocker-r-ver-latest
test-r-rstudio-r-base-4.1-opensuse155
test-r-rstudio-r-base-4.2-focal
test-r-ubuntu-22.04
test-r-versions

kou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

+1

github-actions Bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels May 19, 2025
Comment thread r/src/altrep.cpp
template <typename T>
using Pointer = cpp11::external_pointer<std::shared_ptr<T>, DeletePointer<T>>;

#if ARROW_VERSION_MAJOR >= 18

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Isn't this required for the test-r-arrow-backwards-compatibility job too?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No, we use binaries for that job — though that job is failing for other reasons I need to figure out (https://github.com/ursacomputing/crossbow/actions/runs/15090630647/job/42418790800 is last night's run on main with the same failure)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

github-actions Bot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels May 19, 2025

Copy link
Copy Markdown
Member

After this merges, I'll take a look at rebasing #41998 as it removes the one thing we weren't sure about there

Copy link
Copy Markdown
Member Author

After this merges, I'll take a look at rebasing #41998 as it removes the one thing we weren't sure about there

😦 We've blocked things for ~1 year due to this 😦 That's unfortunate.

But, yes, please! 🙏 I think technically that could merge now since we bumped to 19.0.1 a little while ago already, but waiting until this PR merges is also totally fine.

jonkeane merged commit 218c886 into apache:main May 26, 2025
jonkeane removed the awaiting changes Awaiting changes label May 26, 2025
jonkeane deleted the 43623_remove_compatibility branch May 26, 2025 14:33

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 218c886.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

4 participants


Back | FazBrowse Home | New Git URL