Centralize the handling of browser.OpenURL #7174
Conversation
So we can catch when xdg-open not installed
k8s-ci-robot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afbjorklund The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
There was a problem hiding this comment.
PR looks good just one request, please handle the error gracefully in the caller, instead of crashing, ask user to install the right package
we dont want to direct user to create an issue if we dont want that issue to be created.
💣 open url failed: http://10.128.0.21:31284: exec: "xdg-open": executable file not found in $PATH
😿 minikube is exiting due to an error. If the above message is not useful, open an issue:
|
Right, have to come up with a nice way of handling this. |
Let the terminal user open it themselves
afbjorklund
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
Went with emoji instead |
Codecov Report
@@ Coverage Diff @@
## master #7174 +/- ##
=======================================
Coverage 37.08% 37.08%
=======================================
Files 145 145
Lines 9234 9234
=======================================
Hits 3424 3424
Misses 5387 5387
Partials 423 423
|
| _, err := exec.LookPath("xdg-open") | ||
| if err != nil { | ||
| out.T(out.URL, url) | ||
| return nil |
There was a problem hiding this comment.
how about some additional wording to the URL ?
how does this look like in the output ?
do u mind pasting the ouput in the PR description so we see how it looks like? |
|
I don't have the environment set up to reproduce it, but I guess it would look something like: Hopefully the terminal emulator will make the link clickable for the user, if they connect remotely |
So we can catch when xdg-open not installed
Might help with #7168