MINOR: [R] Add 25.0.1 NEWS entry - #50954
Conversation
There was a problem hiding this comment.
Pull request overview
Adds the missing Arrow R 25.0.1 NEWS entry documenting the macOS mimalloc improvement.
Changes:
- Adds the “Minor improvements and fixes” section.
- References issue #50428.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Nice! |
|
@uros-b Hey there, I've seen you approving various PRs across different areas of the codebase. Just out of curiosity, what's the reason for this? I have concerns that it could either a) cause confusion to new contributors who need their PRs reviewed by a committer and could mix up an approving review by a non-committer with an intention to merge by a committer, or b) lead to committers not reviewing/merging PRs as at first glance it looks like another committer has already reviewed it. I don't think it's technically disallowed, but it's very unusual so I'm curious as to the reasoning. |
|
Yes @thisisnic, I did review the PR here as I think it's a good contribution (hence the approval), but I wouldn't say that I'm trying to interfere or confuse anyone. Regarding a), github UI clearly separates committer and non-committer reviews, so I don't see the confusing part. As for b), committers already use the |
I don't think you have malicious intent, but the reason I'm discouraging it is because it adds noise without any benefit, at a time where there's a lot of noise already from AI contributions etc, and increased pressure on maintainers to manage that. I appreciate your interest in contributing though; happy to point you in the direction of places where it'd be helpful to have extra input if there was a specific area you're most interested in? |
|
I have a different opinion. So I share my opinion here. I think that (non-malicious) reviews from non committers are welcome. (For example, just posting AI generated meaningless reviews are not welcome.) Because:
Our "Becoming a committer" https://arrow.apache.org/committers/#becoming-committer also includes "reviewing other pull requests":
How about mentioning any committer when you approve a PR? We may be able to merge PRs faster. BTW, I think that Nic's this point is a good suggestion too.
|
|
FTR, I agree with @thisisnic's concerns here |
@kou I'm 100% with you for genuine PR reviews where potential contributors are learning, but this is a very unusual pattern of contributions (i.e. not seen before on this repo) that is leading me to conclude it's AI generated or there are other motives at play. And there are cases where it's led to ambiguity, where PRs have been "approved" but actually need further work, e.g. #50837 I'm massively here for supporting newbies asking questions and giving comments on PRs, but look at this one and I'm just backporting a CHANGELOG entry, so it feels like there's something else going on here beyond someone just learning about the codebase. |
|
Thank you @kou and @pitrou for also chiming in as well, although I must also note that some of the comments here are coming off unusually hostile. FWIW, I do in fact intend to focus on one or maybe two areas here and start opening PRs in this repo, but I also want to get to know the codebase and contributors first. Again, I'm not sure whether all of the comments here were really warranted, but I appreciate everyone's time invested in this discussion anyways! All the best |
Rationale for this change
The 25.0.1 section of r/NEWS.md on main is empty; the CRAN 25.0.1 release branch has an entry for the mimalloc change that shipped in the patch release.
What changes are included in this PR?
Adds the 25.0.1 NEWS entry to match the CRAN release branch.
Are these changes tested?
No, documentation only.
Are there any user-facing changes?
No.