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-5383] Updates RegEx to excludes JARs by default #830

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

lukaszlenart
Copy link
Member

Closes WW-5383

&& (alwaysMapExecute || map.isEmpty())) {
&& hasDefaultMethod
&& actionAnnotation == null && actionsAnnotation == null
&& (alwaysMapExecute || map.isEmpty())) {
Copy link
Member

Choose a reason for hiding this comment

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

My personal preference is to use double indents for continuations - it avoids situations like these where the guard blends in with the following code

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using standard IDEA settings, yet it would be good to introduce a common IDE independent mechanism to format code.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO Eclipse does this by default, IntelliJ not.

We could introduce .editorconfig settings. The following setting does the job:

[*.java]
continuation_indent_size = 8

Copy link
Member

Choose a reason for hiding this comment

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

Just checked this as I was curious and doesn't seem to be the case (at least on the version I'm using)
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, right! Just checked my settings and it was 4 for continuation, reverted to default one :)

* can return also a sub-path like "!/META-INF/versions/..."
*/
private static final String EXCLUDE_ALL_JARS_PATTERN = ".*?\\.jar(!/|/)?(.*)?";
private static final String DEFAULT_SPLIT_PATTERN = "\\s*,\\s*";
Copy link
Contributor

Choose a reason for hiding this comment

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

looks good without the original [,] 👍

}
urlSet = urlSet.excludePaths(System.getProperty("sun.boot.class.path", ""));
urlSet = urlSet.exclude(".*/JavaVM.framework/.*");

if (includeJars == null) {
urlSet = urlSet.exclude(".*?\\.jar(!/|/)?");
LOG.debug("\"{}\" is not defined, excluding all JAR files!", ConventionConstants.CONVENTION_ACTION_INCLUDE_JARS);
urlSet = urlSet.exclude(EXCLUDE_ALL_JARS_PATTERN);
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the extra (.*)? from EXCLUDE_ALL_JARS_PATTERN = ".*?\\.jar(!/|/)?(.*)?"; is intentional, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, since JDK9 we can get either jar:file:/.../org/codehaus/plexus/plexus-utils/3.4.1/plexus-utils-3.4.1.jar!/META-INF/versions/10/ or just jar:file:/.../org/codehaus/plexus/plexus-utils/3.4.1/plexus-utils-3.4.1.jar!/

@asfgit asfgit force-pushed the feature/WW-5383-exclude-jars branch from 151970a to cc78033 Compare January 4, 2024 05:49
@lukaszlenart lukaszlenart requested review from sepe81 and kusalk January 4, 2024 05:54
Copy link

sonarqubecloud bot commented Jan 4, 2024

Quality Gate Failed Quality Gate failed

Failed conditions

1 Security Hotspot
69.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

@lukaszlenart lukaszlenart merged commit 5e9dd3d into master Jan 4, 2024
8 of 9 checks passed
@lukaszlenart lukaszlenart deleted the feature/WW-5383-exclude-jars branch January 4, 2024 09:10
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.

3 participants