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.

Sign up for GitHub

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

Jump to bottom

Add missing docs for the rest of the util module #1026

Merged
merged 17 commits into from
Nov 22, 2023
Merged

Add missing docs for the rest of the util module #1026

merged 17 commits into from
Nov 22, 2023

Conversation

qinsoon
Copy link
Member

@qinsoon qinsoon commented Nov 16, 2023

This PR is a step towards #309.

  • Deny missing_docs for the util module and the vm module.
  • Change some items from pub to pub(crate).
  • Remove some unused constants.

@qinsoon qinsoon added the PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead) label Nov 17, 2023
@qinsoon qinsoon mentioned this pull request Nov 18, 2023
Merged
@qinsoon
Copy link
Member Author

qinsoon commented Nov 18, 2023

binding-refs
OPENJDK_BINDING_REPO=qinsoon/mmtk-openjdk
OPENJDK_BINDING_REF=update-mmtk-core-pr-1026

@qinsoon qinsoon mentioned this pull request Nov 19, 2023
Closed
@qinsoon qinsoon requested a review from wks November 20, 2023 09:22
wks
wks requested changes Nov 22, 2023
View reviewed changes
src/util/address.rs Outdated Show resolved Hide resolved
src/util/address.rs Outdated Show resolved Hide resolved
src/util/address.rs Outdated Show resolved Hide resolved
src/util/address.rs Outdated Show resolved Hide resolved
src/util/alloc/bumpallocator.rs Outdated Show resolved Hide resolved
src/util/conversions.rs Outdated Show resolved Hide resolved
src/util/conversions.rs Outdated Show resolved Hide resolved
src/util/conversions.rs Outdated Show resolved Hide resolved
src/util/mod.rs Show resolved Hide resolved
@qinsoon
Copy link
Member Author

qinsoon commented Nov 22, 2023

@wks I have made most of the changes as suggested, and replied to the other comments.

wks
wks approved these changes Nov 22, 2023
View reviewed changes
Copy link
Collaborator

@wks wks left a comment

Choose a reason for hiding this comment

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

LGTM

@qinsoon qinsoon added this pull request to the merge queue Nov 22, 2023
Merged via the queue into mmtk:master with commit b066199 Nov 22, 2023
19 of 20 checks passed
@qinsoon qinsoon deleted the missing-docs-util branch November 22, 2023 12:04
mmtkgc-bot added a commit to mmtk/mmtk-openjdk that referenced this pull request Nov 22, 2023
@qinsoon @mmtkgc-bot
Add Java-specific constants from MMTk (#258)
96e868b 
Java-specific constants in MMTk are no longer public in
mmtk/mmtk-core#1026. This PR duplicates the
constants that are used in the binding.

---------

Co-authored-by: mmtkgc-bot <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Nov 23, 2023
@qinsoon
Add missing docs for the rest of the code base (merge after #1026) (#…
a87636c 
…1028)

This is the last PR for adding missing docs, and it closes
#309. All the public items
should be documented properly now, and `#![deny(missing_docs)]` is used
so any new public item that does not have rustdoc will cause the build
to fail.
@qinsoon qinsoon mentioned this pull request Nov 23, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wks wks wks approved these changes

Assignees
No one assigned
Labels
PR-testing Run binding tests for the pull request (deprecated: use PR-extended-testing instead)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@qinsoon @wks