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

Use okhttp-api-plugin for dependency management #56

Merged
merged 2 commits into from
Jun 16, 2020

Conversation

bitwiseman
Copy link
Contributor

No description provided.

@bitwiseman bitwiseman closed this Jun 15, 2020
@bitwiseman bitwiseman reopened this Jun 15, 2020
@bitwiseman bitwiseman closed this Jun 15, 2020
@bitwiseman bitwiseman reopened this Jun 15, 2020
Copy link
Member

@dwnusbaum dwnusbaum 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 to me, although do you need to add an exclusion on okhttp libraries to the github-api library dependency so that downstream plugins don't see version conflicts (or no because its only an optional dependency in the github-api library)? I only see commons-lang3 and github-api jars being bundled here which seems right, but I wonder if plugins like github-branch-source will be confused because okhttp has two paths in the dependency tree.

@bitwiseman
Copy link
Contributor Author

@dwnusbaum
It hasn't seemed to be a problem. The optional setting of the okhttp dependencies in github-api seem to be handled in correctly - the dependencies from the okhttp-api-plugin win. But it would be better/safer to be explicit about it. I'll make the change.

Optional dependencies are not included in the dependency tree.
However, it is important for this plugin to not bundle the square dependecies from github-api.

This change ensures that we never end up accidently bundling them.
@bitwiseman bitwiseman merged commit a2ac7e3 into jenkinsci:master Jun 16, 2020
@bitwiseman bitwiseman deleted the okhttp-api-plugin branch June 16, 2020 19:33
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