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

kpm - extra slash in default org.killbill.billing.plugin.kpm.nexusUrl #157

Closed
xsalefter opened this issue Mar 2, 2024 · 4 comments
Closed

Comments

@xsalefter
Copy link
Contributor

xsalefter commented Mar 2, 2024

Originally reported here.

Now, AFAIR, this property is a fallback property. But I don't remember why it need to have extra / (that causing bug). Maybe try to comply with previous version?

Nevertheless, current/default state of configuration causing bug explained above, and it need to be fixed. Things that need to do:

  1. After all, constructed URL https://oss.sonatype.org//content/ .... is wrong. It should be construct valid URL.
  2. Removing extra / in default value seems make sense, but do check thoroughly it not break anything else.
  3. Implements this comment.
  4. While working at this: Use consistent naming URI vs URL.
@pierre
Copy link
Member

pierre commented Mar 2, 2024

Related: #156 ?

@xsalefter
Copy link
Contributor Author

Solved by this previous PR.

@xsalefter
Copy link
Contributor Author

Re-open this. I think the fix mentioned by Pierre not covering all problem (see this, or this).

I think we can make this better by:

  1. Define how URL (nexusUrl, and maybe others). should looks like. Add this in README.md or CONFIGURATION.md. For example, it should not end with /: http://sonatype.org and https://repo.maven.org/maven2 is valid but https://repo.maven.org/maven2/ is not.
  2. Validate point 1, in the first time KpmProperties loaded/constructed.
  3. Changes related code to follow this rule (in this case, fix UrlResolverFactory lines as pointed in first paragraph).

@xsalefter xsalefter reopened this Mar 4, 2024
@xsalefter
Copy link
Contributor Author

Fixed by this PR.

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

No branches or pull requests

2 participants