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

Support gradle versions api url customization #807

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Support gradle versions api url customization #807

merged 1 commit into from
Sep 7, 2023

Conversation

sebek64
Copy link
Contributor

@sebek64 sebek64 commented Sep 6, 2023

No description provided.

@sebek64 sebek64 marked this pull request as ready for review September 6, 2023 12:43
@ben-manes
Copy link
Owner

Can you put provide context? Do you have an example of an alternative url, since it is a custom api? Offhand I can’t think of a reason to make this customizable.

@sebek64
Copy link
Contributor Author

sebek64 commented Sep 6, 2023

Thanks for quick reaction.

Yes, the context is maybe a bit non-standard. We want to run this task in a nightly-build runner, which has no direct internet access. So far, we had a proxy, but the next generation of runners have only custom artifactory access, with explicit proxied/mirrored urls. So the real url is mapped to a different one.

In the gradle wrapper properties file, this type of customization is supported, as well as in some other plugins (see for example gradle owasp dependency check plugin and many of its url parameters: http://jeremylong.github.io/DependencyCheck/dependency-check-gradle/configuration.html).

Anyway, it is not an essential feature. If you decide not to merge it, we'll just script it differently, without any big effort.

If you're convinced by the use case, but you want some adjustments in the PR, I'm willing to do that.

@ben-manes
Copy link
Owner

thanks, that seems more reasonable. You'd need to fix the broken tests, maybe add one if possible, and include a one-liner in the readme about the config.

@sebek64
Copy link
Contributor Author

sebek64 commented Sep 7, 2023

I've fixed the failing tests and tuned README. I was thinking about how to write a new test - how to mock the gradle service on an alternative url. There are more ways. One is to start an embedded http server in the test. An alternative can be to put some file to this repo, and refer to this file on github.

It is up to you to chose the right way or suggest another one. Or if we can survive without a test.

@ben-manes
Copy link
Owner

it looks like a large refactoring would be required to mock it with a file, even though that is quite convenient (like we do with the maven repository). Perhaps less so to spin up an http proxy, but that might feel like cumbersome busywork or be flaky. This seems like a small enough change that it's not super important to test more than what we already have, so I'm fine merging as is. We can release it when the tests pass or whenever you're ready to use your enhancement.

@ben-manes ben-manes merged commit 6be5c69 into ben-manes:master Sep 7, 2023
2 checks passed
@sebek64 sebek64 deleted the support-versions-url-customization branch September 7, 2023 07:08
@ben-manes
Copy link
Owner

releasing, should be in the plugin repository in a few minutes.

@sebek64
Copy link
Contributor Author

sebek64 commented Sep 7, 2023

Thank you, looking forward.

@ben-manes
Copy link
Owner

and it's live. Thanks for the contribution!

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