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

[WFLY-15503] Remove duplicate if/else block (weld) #14799

Merged

Conversation

boris-unckel
Copy link
Contributor

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Oct 15, 2021
if (PrivateSubDeploymentMarker.isPrivate(deploymentUnit)
// if any deployments have a beans.xml we need to load portable extensions
// even if this one does not.
&& !WeldDeploymentMarker.isPartOfWeldDeployment(deploymentUnit)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This logical statement is not functionally equivalent to the previous one. With previous logic, a deploymentUnit with PrivateSubDeploymentMarker.isPrivate(deploymentUnit) == false could still be excluded from processing if the second condition was met. However, in your PR, it would be processed.

If you want to collapse this if statement, then it would have to look like this:

if (!WeldDeploymentMarker.isPartOfWeldDeployment(deploymentUnit)) {
    return;
}

(plus keeping the comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@manovotn thanks for your review. Yes, sure, I should have seen that. Fix is ongoing...

@boris-unckel boris-unckel force-pushed the WFLY-15503_remove_dup_ifelse_weld branch from cde97ae to 3657460 Compare October 18, 2021 13:52
@manovotn
Copy link
Contributor

@boris-unckel CI is failing because of checkstyle violation. Otherwise, I am fine with this PR.

@boris-unckel boris-unckel force-pushed the WFLY-15503_remove_dup_ifelse_weld branch from 3657460 to fad6bba Compare October 18, 2021 16:00
@boris-unckel
Copy link
Contributor Author

@manovotn extremely embarrassing. Fixed.

@manovotn
Copy link
Contributor

extremely embarrassing.

Not at all, happens all the time :-)

@bstansberry bstansberry merged commit f11fd29 into wildfly:main Oct 21, 2021
@boris-unckel boris-unckel deleted the WFLY-15503_remove_dup_ifelse_weld branch October 22, 2021 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes
Projects
None yet
3 participants