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

[8.2.x ] ISPN-4390 Realigned with ConcurrentHashMap from the JSR-166 repository #4560

Merged
merged 1 commit into from Oct 13, 2016

Conversation

tristantarrant
Copy link
Member

@tristantarrant tristantarrant commented Sep 16, 2016

https://issues.jboss.org/browse/ISPN-4390

  • jsr166/src/jdk8/java/util/concurrent/ConcurrentHashMap.java 1.3
  • factor out all inner classes/interfaces from BECHMV8
  • refactor access to the Unsafe instace
  • extract the package-level logic from ThreadLocalRandom

@tristantarrant
Copy link
Member Author

@danberindei @wburns I've put the changes on this PR for 8.2.x only. I've factored out the inner classes as requested on the other PR, which I've closed

Copy link
Contributor

@slaskawi slaskawi left a comment

Choose a reason for hiding this comment

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

Since this is a re-alignment I tried not to be picky on the logic (I assume it's correct and there are only minor changes). The only major thing that I found are javadocs. I didn't mark all places, but please add something meaningful or just remove them.

package org.infinispan.commons.util.concurrent.jdk8backported;

/**
* @author Will Burns
Copy link
Contributor

@slaskawi slaskawi Oct 6, 2016

Choose a reason for hiding this comment

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

Do we consider this as a public API? If so, could you please add some Javadoc (or remove it completely if we don't).

Copy link
Member Author

Choose a reason for hiding this comment

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

None of the classes in this package are considered public API

Copy link
Contributor

Choose a reason for hiding this comment

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

So maybe the javadocs should be simply removed... an empty thing is just rubbish...

package org.infinispan.commons.util.concurrent.jdk8backported;

/**
* @author Will Burns
Copy link
Contributor

Choose a reason for hiding this comment

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

Javadocs


import java.util.Map;

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

And here also

import java.util.Collection;

/**
* // TODO: Document this
Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

package org.infinispan.commons.util.concurrent.jdk8backported;

/**
* @author Will Burns
Copy link
Contributor

Choose a reason for hiding this comment

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

Javadocs

- jsr166/src/jdk8/java/util/concurrent/ConcurrentHashMap.java 1.3
- factor out all inner classes/interfaces from BECHMV8
- refactor access to the Unsafe instace
- extract the package-level logic from ThreadLocalRandom
@tristantarrant
Copy link
Member Author

@slaskawi I've removed all the pointless javadocs

@slaskawi
Copy link
Contributor

LGTM, integrating

@slaskawi slaskawi merged commit 65f743f into infinispan:8.2.x Oct 13, 2016
@slaskawi
Copy link
Contributor

Integrated, thanks Tristan! BTW - I updated fix versions for https://issues.jboss.org/browse/ISPN-4390

@tristantarrant tristantarrant deleted the ISPN-4390-8.2.x/jsr166 branch April 10, 2018 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants