#1910: create mvnd url updater#1921
Conversation
MarvMa
added
enhancement
Coverage Report for CI Build 26017516197Warning No base build found for commit Coverage: 70.708%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
satorus
left a comment
satorus
left a comment
There was a problem hiding this comment.
Looks good, tested it on WSL and the urls are updated in my local ide-urls repo. Nice work!
| @Override | ||
| public String mapVersion(String version) { | ||
| // Accept pre-release versions (rc, beta, alpha, etc.) first | ||
| if (version.contains("-rc") || version.contains("-beta") || version.contains("-alpha")) { |
There was a problem hiding this comment.
Surely we want to include the release candidates but should IDEasy really encourage users to use alpha (or beta) releases?
| protected void addVersion(UrlVersion urlVersion) { | ||
| // Support both regular releases and pre-releases (e.g., 2.x versions) | ||
| // This allows users to test the latest versions before they become stable | ||
| String baseUrl = getDownloadBaseUrl() + "${version}/maven-mvnd-${version}-"; |
There was a problem hiding this comment.
I was not aware that getDownloadBaseUrl() guarantees to include a trailing slash.
For paths and URLs it is typically best practice to omit a trailing slash but if you append start with s slash.
String basePath = "/foo/bar";
String path = basePath + "/folder/file.ext";
Just a comment for the record. If this is currently that way, then you did nothing wrong.
Lets simply set this to resolved then and consider a separate PR for a potential cleanup but do not include general cleanups into this PR that should focus on mvnd.
| if (isVersionFiltered()) { | ||
| if (vLower.contains("alpha") || vLower.contains("beta") || vLower.contains("dev") || vLower.contains("snapshot") || vLower.contains("preview") | ||
| || vLower.contains("test") || vLower.contains("tech-preview") // | ||
| || vLower.contains("-pre") || vLower.startsWith("ce-") || vLower.contains("-next") || vLower.contains("-rc") | ||
| // vscode nonsense | ||
| || vLower.startsWith("bad") || vLower.contains("vsda-") || vLower.contains("translation/") || vLower.contains("-insiders")) { |
There was a problem hiding this comment.
why not instead moving the condition to isFilterVersion(String version) so you can override it?
BTW: You are overriding the entire method so this change and the new isVersionFiltered() method is kind of pointless here.
github-project-automation
Bot
moved this from Team Review
to 👀 In review
in IDEasy board
4 participants
This PR fixes #1910
Implemented changes:
MvndUrlUpdaterclass with filteringChecklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal