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

start: use tabs for a Windows command #3215

Merged
merged 6 commits into from
Feb 11, 2022

Conversation

mahnoor-shahid
Copy link
Contributor

@mahnoor-shahid mahnoor-shahid commented Jan 26, 2022

Received and error while removing the data file.

Remove-Item : Parameter cannot be processed because the parameter name 'f' is ambiguous. Possible
matches include: -Filter -Force.

I used Force command to make it work.

Received and error while removing the data file. 

Remove-Item : Parameter cannot be processed because the parameter name 'f' is ambiguous. Possible
matches include: -Filter -Force. 

I used Force command to make it work
@jorgeorpinel
Copy link
Contributor

jorgeorpinel commented Jan 26, 2022

Thanks @mahnoor-shahid but what OS and terminal are you using? -f is correct for GNU/Linux shells which is our assumed platform at the moment.

@shcheklein shcheklein added A: docs Area: user documentation (gatsby-theme-iterative) ⌛ status: wait-response Waiting a response from the outside of the organization labels Jan 26, 2022
@mahnoor-shahid
Copy link
Contributor Author

mahnoor-shahid commented Jan 27, 2022

Windows @jorgeorpinel

@jorgeorpinel
Copy link
Contributor

OK. We will be getting to multi-platform examples in the future per #2939. Meanwhile please also consider these recommended POSIX-like interfaces for Windows: https://dvc.org/doc/user-guide/running-dvc-on-windows

Thanks

@shcheklein
Copy link
Member

may be we could slightly modify specifically this place to use tabs?

@shcheklein
Copy link
Member

no need to all this at once, we could gradually modify them, wdyt, @jorgeorpinel ... could we guide @mahnoor-shahid to slightly change the PR?

@jorgeorpinel
Copy link
Contributor

Sure, the info about how to use tabs is currently in #2939

@jorgeorpinel jorgeorpinel reopened this Feb 2, 2022
Copy link
Contributor

@jorgeorpinel jorgeorpinel left a comment

Choose a reason for hiding this comment

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

This is how you'd use the tab toggle. I'll commit and deploy this to double check... ✔️

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-patch-1-u1nyq9vcgqv1gr February 2, 2022 15:46 Inactive
@jorgeorpinel jorgeorpinel had a problem deploying to dvc-org-patch-1-u1nyq9vcgqv1gr February 2, 2022 15:49 Failure
Comment on lines 175 to 178
<tab title="Windows">

```
> del data\data.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

Separate question: Prism doesn't seem to have a Windows Command Prompt language unfortunately. It does have PowerShell but I assume we're going with plain cmd right @shcheklein ? If so we may need to make a new custom highlighter cc @julieg18 WDYT

Copy link
Contributor

Choose a reason for hiding this comment

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

@jorgeorpinel I don't think we need a custom highlighter since I think PowerShell runs Windows Command Prompt language as well as its own special syntax. Using the powershell language should suffice.

How this script would look in powershell using Prism's Test drive tool:

image

Copy link
Contributor

@jorgeorpinel jorgeorpinel Feb 11, 2022

Choose a reason for hiding this comment

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

Good call @julieg18 ! But in any case the reason we have the dvc highlighter is to highlight dvc {something} commands... So we may need to eventually move that to dvc-posix and make a new dvc-win one, perhaps based of powershell?

We can continue to discuss in #3273

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-patch-1-u1nyq9vcgqv1gr February 2, 2022 15:58 Inactive
@jorgeorpinel
Copy link
Contributor

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-patch-1-u1nyq9vcgqv1gr February 2, 2022 16:04 Inactive
@jorgeorpinel jorgeorpinel self-assigned this Feb 2, 2022
@@ -175,7 +175,7 @@ $ rm -f data/data.xml
<tab title="Windows">

```
> rmdir .dvc/cache
> rmdir .dvc\cache
Copy link
Member

Choose a reason for hiding this comment

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

I would keep $ for now. It's not an important detail to be honest.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.


```
> rmdir .dvc\cache
> del data\data.xml
Copy link
Member

Choose a reason for hiding this comment

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

what about powershell - it it del or does it support Posix?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with PowerShell but it looks like it's a totally different command: Remove-Item.

@jorgeorpinel jorgeorpinel temporarily deployed to dvc-org-patch-1-u1nyq9vcgqv1gr February 10, 2022 16:11 Inactive
@jorgeorpinel jorgeorpinel removed the ⌛ status: wait-response Waiting a response from the outside of the organization label Feb 10, 2022
@shcheklein
Copy link
Member

@iterative/websites

Screen Shot 2022-02-10 at 12 40 34 PM

looks like we need to disable the check for this action?

@julieg18
Copy link
Contributor

julieg18 commented Feb 10, 2022

@iterative/websites

Screen Shot 2022-02-10 at 12 40 34 PM

looks like we need to disable the check for this action?

The action is failing since it only works for prs that aren't forks. Updating the the action, having it run for prs that aren't forks only, should fix the problem.

@shcheklein
Copy link
Member

@julieg18 thanks for looking into this. Should it be a check at all? It's an action that help to run a command and schedule, not sure why do we have / need status for it.

@rogermparent
Copy link
Contributor

Should it be a check at all? It's an action that help to run a command and schedule, not sure why do we have / need status for it.

As far as I understand, certain GH Actions hooks like pull_request are always represented as checks and the behavior cannot be suppressed. It's the same reason link check reports multiple checks, because the "initialize" check runs on pull_request.

@jorgeorpinel
Copy link
Contributor

Thanks for noticing and addressing the check issue. Separately (since you're here), please notice #3215 (review) above. Thanks

p.s. so can we merge this?

@jorgeorpinel jorgeorpinel changed the title Changed line 168 ~ from rm -f data/... to rm -Force data/... start: use tabs for a Windows command Feb 11, 2022
@jorgeorpinel jorgeorpinel mentioned this pull request Feb 11, 2022
12 tasks
@shcheklein shcheklein merged commit b48725a into iterative:master Feb 11, 2022
jorgeorpinel added a commit that referenced this pull request Mar 1, 2022
* start: use `powershell` highligher for tab toggles
per #3215 (comment)

* start: Windows command tabs for Versioning page

* start: use `dvc` highlighter (and `$`) for Windows cmd blocks

per #3273 (review)

* start: Windows code blocks for /tmp location

* Apply suggestions from code review
iesahin pushed a commit that referenced this pull request Apr 11, 2022
* Changed line 168 ~ from rm -f data/... to rm -Force data/...

Received and error while removing the data file. 

Remove-Item : Parameter cannot be processed because the parameter name 'f' is ambiguous. Possible
matches include: -Filter -Force. 

I used Force command to make it work

* Apply suggestions from code review

* Update content/docs/start/data-and-model-versioning.md

* Update content/docs/start/data-and-model-versioning.md

* Update content/docs/start/data-and-model-versioning.md

* Update content/docs/start/data-and-model-versioning.md

Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
iesahin pushed a commit that referenced this pull request Apr 11, 2022
* start: use `powershell` highligher for tab toggles
per #3215 (comment)

* start: Windows command tabs for Versioning page

* start: use `dvc` highlighter (and `$`) for Windows cmd blocks

per #3273 (review)

* start: Windows code blocks for /tmp location

* Apply suggestions from code review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: docs Area: user documentation (gatsby-theme-iterative)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants