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

Deprecated Concurrent::Atomic in lieu of Concurrent::AtomicReference. #277

Merged
merged 1 commit into from
Apr 24, 2015

Conversation

jdantonio
Copy link
Member

See #261

# @!macro atomic_reference
# @deprecated Use Concurrent::AtomicReference instead.
class Concurrent::Atomic < Concurrent::AtomicReference
end
Copy link
Member

Choose a reason for hiding this comment

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

Do we need aliases for the different implementations? In my mind they are not included in the public API. Just assignment Atomic = AtomicReference might be better since it clearly says what is the main name and the alias will also correctly report full name AtomicReference.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@jdantonio jdantonio force-pushed the namespacing branch 2 times, most recently from 72c199c to 05788e9 Compare April 23, 2015 11:44
@jdantonio
Copy link
Member Author

The test that failed in the last run is one that is known to fail intermittently. See #271

# @!macro atomic_reference
# @deprecated Use Concurrent::AtomicReference instead.
class Concurrent::Atomic < Concurrent::AtomicReference
end
Copy link
Member

Choose a reason for hiding this comment

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

I meant something like:

module Concurrent
  Atomic = AtomicReference
end

So it is actually the same class, because having two classes with same behavior but not-identical may be causing problems in comparisons when both names are used.

Copy link
Member Author

Choose a reason for hiding this comment

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

When I've tried to do that in the past yardoc wouldn't generate documentation for the 'alias' class. That's the main reason I did the subclass. What you say makes sense, though. I'll make the change and then figure out some way to get reasonable documentation in yardoc.

@pitr-ch
Copy link
Member

pitr-ch commented Apr 23, 2015

@jdantonio Are you able to restart the jobs on travis? It does not work for me :(

@jdantonio
Copy link
Member Author

I was able to restart it, but I had to log in to Travis in order to make the "restart" button visible. Interestingly, Travis says that the build has restarted, but I can't tell if it actually has.

@pitr-ch
Copy link
Member

pitr-ch commented Apr 23, 2015

Thanks, I saw it but it was doing nothing :/

jdantonio added a commit that referenced this pull request Apr 24, 2015
Deprecated Concurrent::Atomic in lieu of Concurrent::AtomicReference.
@jdantonio jdantonio merged commit 9f897f3 into master Apr 24, 2015
@jdantonio jdantonio deleted the namespacing branch April 24, 2015 11:20
@jdantonio jdantonio mentioned this pull request Apr 24, 2015
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

Successfully merging this pull request may close these issues.

None yet

2 participants