Add ability to install runtime only #18
Conversation
nikolai-laevskii
force-pushed
the
install-runtime-only
branch
from
60fb674
to
ff40a49
Compare
| dotnetInstaller = new DotnetCoreInstaller(version, quality); | ||
| dotnetVersionResolver = new DotnetVersionResolver(version); | ||
| dotnetInstaller = new DotnetCoreInstaller( | ||
| await dotnetVersionResolver.createDotnetVersion(), |
There was a problem hiding this comment.
I approved the whole PR but still have a question: what was a reason to move DotnetVersionResolver out of DotnetCoreInstaller? It doesn't seem to use any benefits of being instantiated in the run but brings a bunch of not-necessary changes in the tests.
There was a problem hiding this comment.
In this particular PR it was done primarily to keep it in sync with the other PR: #19 so there wouldn't be so many conflicts to resolve and they wouldn't be too complicated. In general, syncing them like that, might not be a good idea, but because setup-dotnet doesn't get that many updates, it's just makes things a bit easier without any significant drawbacks.
As per change itself, it removes hard-coupled dependency on DotnetVersionResolver and makes implementation more flexible when it comes to introducing changes.
- DotnetVersionResolver can now have additional parameters and they won't have to be passed through DotnetCoreInstaller
- DotnetVersionResolver logic and API can now be altered without introducing changes to DotnetCoreInstaller
- DotnetCoreInstaller doesn't need to have version resolution logic inside it as it only works with specific version format. Removing such logic makes DotnetCoreInstaller simpler and easier to maintain.
|
This switch is cool, but I have another opposite idea, add a |
Description:
Added ability to select to install runtime only instead of SDK. It utilizes built-in features of the dotnet-install script by conditionally passing -Runtime/--runtime flag.
Related issue:
actions#352
Check list: