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

CAtomicFixnum should probably be a private constant #981

Open
eregon opened this issue Jan 8, 2023 · 1 comment
Open

CAtomicFixnum should probably be a private constant #981

eregon opened this issue Jan 8, 2023 · 1 comment

Comments

@eregon
Copy link
Collaborator

eregon commented Jan 8, 2023

Otherwise we get misleading suggestions from did-you-mean in case concurrent/atomic/atomic_fixnum is not required but the extension is loaded like:

     NameError:
       uninitialized constant Concurrent::AtomicFixnum
     
                 counter = AtomicFixnum.new
                           ^^^^^^^^^^^^
       Did you mean?  Concurrent::CAtomicFixnum
     # ./spec/concurrent/atomic/cyclic_barrier_spec.rb:106:in `block (4 levels) in <module:Concurrent>'

Note that even if it's private, did-you-mean is still kind of misleading:

     NameError:
       uninitialized constant Concurrent::AtomicFixnum
     
                   counter = AtomicFixnum.new
                             ^^^^^^^^^^^^
       Did you mean?  Concurrent::AtomicReference
     # ./spec/concurrent/atomic/cyclic_barrier_spec.rb:191:in `block (5 levels) in <module:Concurrent>'

And there are various specs checking defined? Concurrent::CAtomicFixnum and even a benchmark referencing Concurrent::CAtomicFixnum, so it's not so easy to make it private unfortunately.

Unclear if worth fixing. WIP at master...eregon:concurrent-ruby:private-AtomicFixnum

@eregon
Copy link
Collaborator Author

eregon commented Jan 23, 2023

Given #986, it seems important to communicate very clearly which classes are internal, e.g., using private_constant.
Apparently "a class is only public if present in the documentation/README" is not clear enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant