Skip to content

GH-50087: [Docs][C++] [Docs][C++] Fix sentence structure in memory.rst regarding MemoryManager#50324

Open
OmBiradar wants to merge 1 commit into
apache:mainfrom
OmBiradar:fix-docs-memorypool
Open

GH-50087: [Docs][C++] [Docs][C++] Fix sentence structure in memory.rst regarding MemoryManager#50324
OmBiradar wants to merge 1 commit into
apache:mainfrom
OmBiradar:fix-docs-memorypool

Conversation

@OmBiradar

@OmBiradar OmBiradar commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

In file docs/source/cpp/memory.rst:

  • Removes a duplicate line
  • Minor grammar fix

Rationale for this change

I believe the statement

The associated class :class:arrow::MemoryManager specifies how to allocate on a given device.

was repeated twice and there was some grammatical mistake in the same paragraph

* Removes a duplicate line
* Minor grammer fix

Signed-off-by: OmBiradar <ombiradar04@gmail.com>
@OmBiradar OmBiradar requested a review from pitrou as a code owner July 1, 2026 18:00
Copilot AI review requested due to automatic review settings July 1, 2026 18:00
@github-actions github-actions Bot added the awaiting review Awaiting review label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR cleans up the C++ memory documentation by removing a duplicated sentence fragment in the arrow::Device / arrow::MemoryManager explanation, improving readability without changing meaning.

Changes:

  • Removed a duplicated/redundant description of arrow::MemoryManager allocation behavior.
  • Left the remaining paragraph with a single, clear example referencing arrow::MemoryPool on the CPU.

@OmBiradar

OmBiradar commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

cc: @amoeba

@amoeba amoeba changed the title GH-50087: [DOCS] [MINOR]Fix the docs of arrow::MemoryManager GH-50087: [Docs][C++] Fix the docs of arrow::MemoryManager Jul 1, 2026
@amoeba

amoeba commented Jul 1, 2026

Copy link
Copy Markdown
Member

Hey @OmBiradar, thanks for the PR. The patch looks good.

Before we approve and merge, could you make some changes to the issue title and PR title to help maintainers and future contributors out?

  • I already changed the PR title a bit:
    • I changed [DOCS] to [Docs] just to match the style used in other PRs. Using existing issues and PRs as a guide for new ones is often a good idea.
    • I removed the [MINOR] tag since we don't use that tag when the PR has an associated issue. Filing an issue like you did is always a good step if you aren't sure but, now that you have some more experience, changes like this are fine as minor PRs and filing an issue first isn't required.
    • I added [C++] since it helps to clarify this Docs change is to the C++ docs.
  • Can you improve the wording of the PR title? "Fix the docs of arrow::MemoryManager" isn't specific enough. PR titles will show up in the git blame so something specific would be better. For example, "Remove misplaced sentence fragment in memory.rst"
  • Can you improve the issue title? Something more specific (like above) would be good.

@OmBiradar OmBiradar changed the title GH-50087: [Docs][C++] Fix the docs of arrow::MemoryManager GH-50087: [Docs][C++] [Docs][C++] Fix sentence structure in memory.rst regarding MemoryManager Jul 2, 2026
@OmBiradar

OmBiradar commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Hey @amoeba I have changed the titles of both the PR and the Issue. Let me know if its ok.

I will keep in mind the points you mentioned in all my future PR's and issues raised.

Thanks for the help!

@OmBiradar OmBiradar changed the title GH-50087: [Docs][C++] [Docs][C++] Fix sentence structure in memory.rst regarding MemoryManager GH-50087: [Docs][C++] [Docs][C++] Fix sentence structure in memory.rst regarding MemoryManager Jul 2, 2026
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.

3 participants