Skip to content
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

Add some documentation for memory_resource #1217

Merged
merged 10 commits into from
Jan 19, 2024

Conversation

miscco
Copy link
Collaborator

@miscco miscco commented Dec 15, 2023

Currently there is no documentation for our memory resource design.

@miscco
Copy link
Collaborator Author

miscco commented Dec 15, 2023

@harrism I cannot add you to the reviewers, so here a ping

@miscco miscco added feature request New feature or request. libcu++ For all items related to libcu++ area: docs labels Dec 15, 2023
@harrism
Copy link
Contributor

harrism commented Dec 17, 2023

@jrhemstad can you change it so I can be added as a reviewer?

Copy link
Collaborator

@griwes griwes left a comment

Choose a reason for hiding this comment

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

Please also wrap the lines of text at some reasonable column.

Otherwise, looking good other than a few points.

libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
Copy link
Contributor

@harrism harrism left a comment

Choose a reason for hiding this comment

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

praise: Wow, great docs with fantastic examples. I love the advice at the end.

suggestion: Since there is a lot in here, perhaps a TOC or some guide to the doc at the top would be useful?

question/suggestion: This is a great didactic document. But it is not an API reference. Is there an API reference to the concepts and refs?

nit: please correct the spelling of "documentation" in the PR title.

libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
@miscco miscco changed the title Add some docummentation for memory_resource Add some documentation for memory_resource Dec 21, 2023
Copy link
Contributor

@harrism harrism left a comment

Choose a reason for hiding this comment

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

I like the reorg. Just a few more suggestions.

libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
libcudacxx/docs/extended_api/memory_resource.md Outdated Show resolved Hide resolved
Co-authored-by: Mark Harris <783069+harrism@users.noreply.github.com>
Copy link
Contributor

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Looks good!

@miscco miscco requested a review from griwes January 15, 2024 12:35
Co-authored-by: Mark Harris <783069+harrism@users.noreply.github.com>
@miscco
Copy link
Collaborator Author

miscco commented Jan 17, 2024

@griwes you are still marked as "changes requested" could you give this another look?

@miscco miscco enabled auto-merge (squash) January 18, 2024 22:06
@miscco miscco disabled auto-merge January 18, 2024 22:17
Co-authored-by: Jake Hemstad <jhemstad@nvidia.com>
@miscco miscco enabled auto-merge (squash) January 18, 2024 22:42
@miscco miscco merged commit 9e3516d into NVIDIA:main Jan 19, 2024
537 checks passed
@miscco miscco deleted the memory_resource_docs branch January 19, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs feature request New feature or request. libcu++ For all items related to libcu++
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants