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.

Sign up for GitHub

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

Jump to bottom

plots: Use URI for --open. #6845

Merged
merged 1 commit into from
Oct 29, 2021
Merged

plots: Use URI for --open. #6845

merged 1 commit into from
Oct 29, 2021

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented Oct 20, 2021

Closes #6092

@daavoo daavoo requested review from pared and skshetry October 20, 2021 21:32
@daavoo daavoo requested a review from a team as a code owner October 20, 2021 21:32
@daavoo daavoo changed the title Use URL for plots show open Use URI for plots show open Oct 20, 2021
@daavoo
Copy link
Contributor Author

daavoo commented Oct 20, 2021

@skshetry I can't really test WSL but it looks like the patch for WSL introduced in #5670 was actually reverted in #6431 ?

@pared
Copy link
Contributor

pared commented Oct 21, 2021

@skshetry I can't really test WSL but it looks like the patch for WSL introduced in #5670 was actually reverted in #6431

my bad, sorry

pared
pared suggested changes Oct 21, 2021
View reviewed changes
dvc/command/plots.py Outdated Show resolved Hide resolved
@daavoo daavoo requested a review from pared October 21, 2021 10:49
pared
pared approved these changes Oct 21, 2021
View reviewed changes
dvc/command/plots.py Show resolved Hide resolved
@daavoo daavoo self-assigned this Oct 25, 2021
@daavoo daavoo added the bugfix fixes bug label Oct 25, 2021
@daavoo
plots: Use URI for --open.
ed59d7e 
We were previously using relpath in *all* cases, mainly to support WSL but relpath doesn't work
on macOS.

Added a check for using relpath *only* for WSL.

Fixes #6092
@daavoo daavoo force-pushed the plots-show-open-url branch from f41ba76 to ed59d7e Compare October 27, 2021 18:08
@daavoo daavoo changed the title Use URI for plots show open plots: Use URI for --open. Oct 27, 2021
@daavoo daavoo changed the title plots: Use URI for --open. plots: Use URI for --open. Oct 27, 2021
@daavoo daavoo added the A: plots Related to the plots label Oct 27, 2021
@pared pared merged commit 4dd5441 into master Oct 29, 2021
@pared pared deleted the plots-show-open-url branch October 29, 2021 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pared pared pared approved these changes

@skshetry skshetry Awaiting requested review from skshetry

Assignees

@daavoo daavoo

Labels
A: plots Related to the plots bugfix fixes bug
Projects
No open projects
DVC
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

plots --open: not working on macOS
2 participants
@daavoo @pared