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

WFLY-15725 Replace duplicate ThreadLocalStack classes with a common one #15398

Merged
merged 1 commit into from Apr 21, 2022
Merged

WFLY-15725 Replace duplicate ThreadLocalStack classes with a common one #15398

merged 1 commit into from Apr 21, 2022

Conversation

RanabirChakraborty
Copy link
Contributor

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Apr 4, 2022
@RanabirChakraborty RanabirChakraborty marked this pull request as draft April 5, 2022 06:51
@RanabirChakraborty RanabirChakraborty marked this pull request as ready for review April 6, 2022 07:24
@chengfang
Copy link
Contributor

@RanabirChakraborty can you change the PR title to the issue synopsis? same for the commit message.

When the PR is ready, can you restart the CI run?

@RanabirChakraborty RanabirChakraborty changed the title WFLY-15725 https://issues.redhat.com/browse/WFLY-15725 WFLY-15725 Replace duplicate ThreadLocalStack classes with a common one Apr 7, 2022
bstansberry
bstansberry requested changes Apr 13, 2022
View reviewed changes
ee/src/main/java/org/jboss/as/ee/concurrent/ConcurrentContext.java Outdated
@@ -26,7 +26,7 @@
import org.jboss.as.ee.concurrent.handle.SetupContextHandle;
import org.jboss.as.ee.logging.EeLogger;
import org.jboss.as.ee.concurrent.handle.ContextHandleFactory;
import org.jboss.as.naming.util.ThreadLocalStack;
import org.wildfly.common.function.ThreadLocalStack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this import to the new correct alphabetical location.

ejb3/src/main/java/org/jboss/as/ejb3/context/CurrentInvocationContext.java Outdated
@@ -24,7 +24,7 @@
import org.jboss.as.ee.component.ComponentInstance;
import org.jboss.as.ejb3.logging.EjbLogger;
import org.jboss.as.ejb3.component.EjbComponentInstance;
import org.jboss.as.ejb3.util.ThreadLocalStack;
import org.wildfly.common.function.ThreadLocalStack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this import to the new correct alphabetical location.

jpa/subsystem/src/main/java/org/jboss/as/jpa/container/NonTxEmCloser.java Outdated
@@ -26,7 +26,7 @@

import java.util.HashMap;
import java.util.Map;

import org.wildfly.common.function.ThreadLocalStack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this import to the new correct alphabetical location.

naming/src/main/java/org/jboss/as/naming/WritableServiceBasedNamingStore.java Outdated
@@ -26,7 +26,7 @@
import org.jboss.as.naming.deployment.RuntimeBindReleaseService;
import org.jboss.as.naming.logging.NamingLogger;
import org.jboss.as.naming.service.BinderService;
import org.jboss.as.naming.util.ThreadLocalStack;
import org.wildfly.common.function.ThreadLocalStack;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this import to the new correct alphabetical location.

@RanabirChakraborty
Copy link
Contributor Author

RanabirChakraborty commented Apr 13, 2022 •
edited

@bstansberry did the changes accordingly.
jpa subsystem is building successfully. Tried to add the wildfly-common as dependencies inside testsuite but still no luck. If possible can you please take a look why NonTxEmCloser is not able to get ThreadLocalStack class from wildlfy-common repo.

@chengfang
Copy link
Contributor

@RanabirChakraborty did you add wildfly-common as a dep in module.xml for the affected modules, if wildfly-common is not already there?
I think it's a runtime dependency problem.

bstansberry
bstansberry reviewed Apr 19, 2022
View reviewed changes
jpa/subsystem/src/main/java/org/jboss/as/jpa/container/NonTxEmCloser.java Outdated
import java.util.HashMap;
import java.util.Map;

import javax.persistence.EntityManager;
import static org.jboss.as.jpa.messages.JpaLogger.ROOT_LOGGER;
Copy link
Contributor

Choose a reason for hiding this comment

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

@RanabirChakraborty The 'import static' should be first.

Our code style is:

import static

java.* (alphabetical)
javax.* (alphabetical)

other.* (alphabetical)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! @bstansberry did the changes.

bstansberry
bstansberry requested changes Apr 19, 2022
View reviewed changes
Copy link
Contributor

@bstansberry bstansberry left a comment

Choose a reason for hiding this comment

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

One last import ordering thing and this should be good to merge.

@bstansberry bstansberry merged commit ea5803d into wildfly:main Apr 21, 2022
@bstansberry
Copy link
Contributor

Thanks, @RanabirChakraborty

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bstansberry bstansberry bstansberry approved these changes

Assignees
No one assigned
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
Milestone
No milestone
3 participants
@RanabirChakraborty @chengfang @bstansberry