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

#690: add support for rancher desktop #727

Merged
merged 24 commits into from
May 24, 2022

Conversation

cinnamon-coder-hub
Copy link
Member

@cinnamon-coder-hub cinnamon-coder-hub commented Apr 19, 2022

fixing #690.

  • Additionally an issue with the mirror has to be and will be fixed soon. There is a formating problem when uploading to github, similar as @CREITZ25 experienced once (PR waiting for merge).

@github-actions github-actions bot added bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) documentation related to documentation (AsciiDoc) scripts related to shell scripts (bash and CMD) labels Apr 19, 2022
Copy link
Member

@tobka777 tobka777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I tested rancher for mac and your solution works with hdiutil.
Instead of .dmg you could also use the zip (https://github.com/rancher-sandbox/rancher-desktop/releases/download/v1.2.1/Rancher.Desktop-1.2.1-mac.x86_64.zip) file, but that's fine. Then you would not need to mount the dmg.
I still had to adjust the name of the .dmg.

scripts/src/main/resources/scripts/command/docker Outdated Show resolved Hide resolved
@hohwille hohwille changed the title 690 cmdlet rancher desktop #690: cmdlet rancher desktop May 2, 2022
@hohwille hohwille changed the title #690: cmdlet rancher desktop #690: add support for rancher desktop May 2, 2022
scripts/src/main/resources/scripts/command/docker Outdated Show resolved Hide resolved
documentation/docker.asciidoc Outdated Show resolved Hide resolved
scripts/src/main/resources/scripts/command/docker Outdated Show resolved Hide resolved
@hohwille hohwille linked an issue May 6, 2022 that may be closed by this pull request
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cinnamon-coder-hub Excellent. Thanks for your updates 👍
We should surely do more testing and collect early user-feedback but now approved and ready to be merged.

@cinnamon-coder-hub cinnamon-coder-hub added this to the release:2022.04.001 milestone May 9, 2022
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We create extra issues for doc improvements (installation help, screenshot of install options) and improvement for #742 as well as MacOS testing - rest is fine so ready for merge

Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cinnamon-coder-hub I did some constructive review with 2 direct commits of small and formal improvements.
Further I just wanted to get this ready for merge but found two more small things that I added as review comments. Please have a look.

scripts/src/main/resources/scripts/command/docker Outdated Show resolved Hide resolved
scripts/src/main/resources/scripts/command/docker Outdated Show resolved Hide resolved
Copy link
Member

@hohwille hohwille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cinnamon-coder-hub Thanks for all the rework and great that you could already integrate vpnkit. I consider this as ready for merge. Great job 👍

@hohwille
Copy link
Member

Shellcheck still fails:

In scripts/command/docker line 157:
  partialUrl=$(curl -Ls -o /dev/null -w %{url_effective} https://github.com/sakai135/wsl-vpnkit/releases/latest)
                                         ^-- SC1083: This { is literal. Check expression (missing ;/\n?) or quote it.
                                                       ^-- SC1083: This } is literal. Check expression (missing ;/\n?) or quote it.


In scripts/command/docker line 173:
  echo 'mv wsl-vpnkit.tar.gz $env:USERPROFILE' >> vpnkit-setup.ps1
  ^-- SC2129: Consider using { cmd1; cmd2; } >> file instead of individual redirects.
       ^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.


In scripts/command/docker line 175:
  echo 'pushd $env:USERPROFILE' >> vpnkit-setup.ps1
       ^----------------------^ SC2016: Expressions don't expand in single quotes, use double quotes for that.


In scripts/command/docker line 176:
  echo 'wsl --import wsl-vpnkit $env:USERPROFILE\wsl-vpnkit wsl-vpnkit.tar.gz --version 2' >> vpnkit-setup.ps1
       ^-- SC2016: Expressions don't expand in single quotes, use double quotes for that.

I will quickly fix this oneline to get this merged and do release today...

@hohwille hohwille merged commit 15c1328 into devonfw:master May 24, 2022
@hohwille hohwille linked an issue May 24, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) documentation related to documentation (AsciiDoc) scripts related to shell scripts (bash and CMD)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add VPN enhancement to RancherDesktop Support for RancherDesktop
3 participants