#1942: create ruby url updater#2066
Conversation
Caylipp
added
enhancement
Coverage Report for CI Build 28951267785Coverage decreased (-0.02%) to 72.104%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions15 previously-covered lines in 3 files lost coverage.
Coverage Stats
💛 - Coveralls |
|
I followed the testing instructions and everything has worked as expected. Well-implemented, properly tested 👍 🎉. |
| String baseUrl = createGithubReleaseDownloadUrl("RubyInstaller-${version}", "rubyinstaller-"); | ||
|
|
||
| doAddVersion(urlVersion, baseUrl + "x64.7z", WINDOWS, X64); | ||
| } |
There was a problem hiding this comment.
We are only addressing Windows here so Linux and Mac users will fail to install ruby?
I am again shocked that the ruby guys only provide their source-code and ask the users to compile it themselves:
https://www.ruby-lang.org/en/downloads/
See
https://github.com/devonfw/IDEasy/blob/main/documentation/tool-vendor-plea.adoc#binary-releases
So if there are no releases for MacOS (and Linux) should we really integrate ruby into IDEasy?
This will only cause headaches if then projects think it is supported and start using it but then Mac/Linux users joining the team will be left in the rain.
Why do tool vendors not do their homework and this has always to be that hard?
There was a problem hiding this comment.
I see your point. This could definitely cause issues later on. If Ruby support is highly requested, we could document that only Windows is supported for now and that macOS/Linux users need to install Ruby manually. Otherwise, I am not sure if the integration effort is worth it.
| public class RubyUrlUpdaterMock extends RubyUrlUpdater { | ||
|
|
||
| private final String baseUrl; | ||
|
|
||
| private final WireMockRuntimeInfo wmRuntimeInfo; | ||
|
|
||
| RubyUrlUpdaterMock(WireMockRuntimeInfo wireMockRuntimeInfo) { | ||
| super(); | ||
| this.wmRuntimeInfo = wireMockRuntimeInfo; | ||
| this.baseUrl = wireMockRuntimeInfo.getHttpBaseUrl(); | ||
| } | ||
|
|
||
| @Override | ||
| protected String getDownloadBaseUrl() { | ||
| return this.baseUrl; | ||
| } | ||
|
|
||
| @Override | ||
| protected String doGetVersionUrl() { | ||
| return this.baseUrl + "/repos/" + getGithubOrganization() + "/" + getGithubRepository() + "/releases"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Thanks for the hint.
There was a problem hiding this comment.
When you have resolved this and created an issue to support ruby for linux/mac, we can merge this PR.
There was a problem hiding this comment.
Due to no reaction, I have to take this out of the release.
There was a problem hiding this comment.
Thanks for the feedback and sorry for the delayed reaction.
I removed the RubyUrlUpdaterMock and adapted RubyUrlUpdater.
I will also create a separate issue to support Ruby for Linux/macOS.
- change min version - add testcase for true negative
Caylipp
force-pushed
the
feature/#1942-create-ruby-url-updater
branch
from
9c2494b to
a60a493
Compare
4 participants
This PR fixes #1942
Implemented changes:
2.5.3-1because older releases use unsupported legacy tag naming.Testing instructions
Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:
mvn clean test.RubyUrlUpdaterTestpasses.2.5.3-1creates astatus.jsonand awindows_x64URL entry.2.4.0-7is ignored.Checklist 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