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

WW-5391 Add interface for VelocityManager extension point #867

Merged
merged 5 commits into from
Feb 5, 2024

Conversation

kusalk
Copy link
Member

@kusalk kusalk commented Feb 2, 2024

WW-5391

Allows applications to provide a completely custom VelocityManager


public void setVelocityManager(VelocityManagerInterface vmi) {
LOG.trace("OSGi ConfigurationProvider - setVelocityManager() called - VelocityManager: [{}]", vmi);
if (!(vmi instanceof VelocityManager)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Didn't bother with a marker interface here as this plugin is deprecated anyway

Copy link
Member Author

Choose a reason for hiding this comment

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

Standardised line endings to LF

/**
* @since 6.4.0
*/
public interface VelocityManagerInterface {
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe in 7.0 we can consider renaming VelocityManager to StrutsVelocityManager and VelocityManagerInterface to VelocityManager

@kusalk kusalk marked this pull request as draft February 2, 2024 06:33
@kusalk kusalk marked this pull request as ready for review February 2, 2024 06:51
@kusalk kusalk requested a review from lukaszlenart February 2, 2024 06:52
Copy link
Member

@lukaszlenart lukaszlenart left a comment

Choose a reason for hiding this comment

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

Looks good, are you going to address some of the reported issues by Sonar?

@kusalk
Copy link
Member Author

kusalk commented Feb 2, 2024

@lukaszlenart Sonar results don't make any sense, it's been misconfigured ever since the 7.0 branch was introduced

@lukaszlenart
Copy link
Member

Should be fixed

@kusalk
Copy link
Member Author

kusalk commented Feb 3, 2024

@lukaszlenart Thanks! Looks like it's just complaining about deprecations, serialisation and code coverage. I've fixed the serialisation warnings. The code coverage I don't think it makes sense for me to rectify as I've just done a find and replace on injection methods. Adding coverage to those injection methods alone won't add much value, and it's out of scope for me to add coverage to the whole class.

Copy link

sonarqubecloud bot commented Feb 3, 2024

Quality Gate Failed Quality Gate failed

Failed conditions

8.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@kusalk kusalk merged commit 4aa1cbc into master Feb 5, 2024
8 of 9 checks passed
@kusalk kusalk deleted the WW-5391-velocity-ext-point branch February 5, 2024 02:39
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.

2 participants