KEMBAR78
Properly document Ractor#require by zverok · Pull Request #12389 · ruby/ruby · GitHub
Skip to content

Properly document Ractor#require #12389

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
Merged
merged 1 commit into from
Dec 21, 2024
Merged

Properly document Ractor#require #12389

merged 1 commit into from
Dec 21, 2024

Conversation

@zverok
Copy link
Contributor

@zverok zverok commented Dec 18, 2024 •
edited
Loading

Currently, the docs have:

  • internal methods Ractor._activated and Ractor._require included;
  • (metaprogrammed) Ractor.require wrongly recognized by RDoc as class method (with no documentation).

This PR just adds `:nodoc: for those methods.

@zverok zverok added the Documentation Improvements to documentation. label Dec 18, 2024
@zverok zverok requested review from jeremyevans and ko1 December 18, 2024 20:10
@zverok zverok self-assigned this Dec 18, 2024
ko1
ko1 reviewed Dec 18, 2024
View reviewed changes
jeremyevans
jeremyevans reviewed Dec 18, 2024
View reviewed changes
Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

I think the :nodoc: is fine, but I'm not sure we need to document the behavior. It's an implementation detail, and the only change is that something that didn't work when you would expect it to, now works the way you expect. If you really think this should be documented, doc/ractor.md seems like a good place, as @ko1 mentioned.

@zverok
Copy link
Contributor Author

zverok commented Dec 19, 2024

I think the :nodoc: is fine, but I'm not sure we need to document the behavior.

Yeah, you are right, we should not. I thought/experimented a bit more and understood that my documentation was completely incorrect (it is not Ractor#require at all, it is Kernel#require is adjusted when the first ractor is activated).

So the PR is just a bunch of `:nodocs: now. It would be good to explain the design decisions once, but for now I am concerned about all the user-faced docs to adjust before the release.

@zverok zverok requested review from jeremyevans and ko1 December 19, 2024 19:26
@zverok zverok force-pushed the document-ractor-new-methods branch from 153413e to 00808da Compare December 21, 2024 17:54
@zverok zverok merged commit efe671f into ruby:master Dec 21, 2024
42 checks passed
@zverok zverok deleted the document-ractor-new-methods branch December 21, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeremyevans jeremyevans jeremyevans approved these changes

@ko1 ko1 ko1 approved these changes

Assignees

@zverok zverok

Labels

Documentation Improvements to documentation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zverok @jeremyevans @ko1