Skip to content

Making Linkage Checker plugin more robust#2178

Merged
suztomo merged 5 commits intomasterfrom
robust
Aug 11, 2021
Merged

Making Linkage Checker plugin more robust#2178
suztomo merged 5 commits intomasterfrom
robust

Conversation

@suztomo
Copy link
Contributor

@suztomo suztomo commented Aug 11, 2021

Rather than failing completely upon unexpected values/conditions, this PR shows the partial results.

Screen Shot 2021-08-11 at 17 09 31

@google-cla google-cla bot added the cla: yes label Aug 11, 2021
try {
classPathResult.formatDependencyPaths(ImmutableList.of(jarB));
fail("The irrelevant JAR file should be invalidated.");
} catch (IllegalArgumentException expected) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this PR, the method doesn't throw IllegalArgumentException any more.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The warning is not critical message. I keep as it is.

try {
classPathResult.formatDependencyPaths(ImmutableList.of(jarB));
fail("The irrelevant JAR file should be invalidated.");
} catch (IllegalArgumentException expected) {
Copy link
Contributor

Choose a reason for hiding this comment

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

"Circular dependency for: "
+ resolvedDependencyResult
+ "\n The stack is: "
+ ImmutableList.copyOf(stack));
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably don't need the defensive copy here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right. Removed.

@suztomo suztomo merged commit ce651b7 into master Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants