Skip to content

Add docs about pager in the pipeline command#831

Merged
shcheklein merged 18 commits into
masterfrom
unknown repository
Dec 11, 2019
Merged

Add docs about pager in the pipeline command#831
shcheklein merged 18 commits into
masterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Dec 2, 2019

Fixes #836

@shcheklein

This comment has been minimized.

Comment thread static/docs/command-reference/pipeline/show.md Outdated
@ghost

This comment has been minimized.

@ghost ghost marked this pull request as ready for review December 8, 2019 13:28
@ghost ghost changed the title Add docs about pager in the pipeline command WIP Add docs about pager in the pipeline command Dec 8, 2019
@ghost ghost changed the title WIP Add docs about pager in the pipeline command Add docs about pager in the pipeline command Dec 8, 2019
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
Copy link
Copy Markdown
Contributor

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

Good stuff!

There are other places that explain navigation by W, A, etc - please do git grep

Also, check some comments I left.

Tymoteusz Jankowski added 3 commits December 9, 2019 20:57
1. add dvc or bash to code blocks
2. remove a reference of WASD
3. replace a code comment with a paragraph
@shcheklein shcheklein temporarily deployed to dvc-landing-2807-use-pa-emgsrg December 9, 2019 20:26 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-2807-use-pa-emgsrg December 9, 2019 21:12 Inactive
@ghost

This comment has been minimized.

@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 9, 2019

@xliiv build is failing, pre-commit is not installed? check the docs contributing guide, please to install it

I installed it yesterday.. i'm not sure why the invalid commit passed.. .
Maybe it's related to vim's fugitive where i do commits?

Copy link
Copy Markdown
Contributor

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

👍

Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Copy link
Copy Markdown
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.

Some specific notes below. Please also spell check all your changes (I fixed a few typos).

Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
@shcheklein shcheklein temporarily deployed to dvc-landing-2807-use-pa-emgsrg December 11, 2019 19:43 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-2807-use-pa-emgsrg December 11, 2019 19:55 Inactive
@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 11, 2019

Some specific notes below. Please also spell check all your changes (I fixed a few typos).

good catch, checked and fixed two words.. 👍

Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
Copy link
Copy Markdown
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.

Almost there. Thanks!

@shcheklein shcheklein temporarily deployed to dvc-landing-2807-use-pa-emgsrg December 11, 2019 20:29 Inactive
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/command-reference/pipeline/show.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
@shcheklein shcheklein temporarily deployed to dvc-landing-2807-use-pa-emgsrg December 11, 2019 20:39 Inactive
Copy link
Copy Markdown
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.

Last details I think

Comment thread static/docs/user-guide/running-dvc-on-windows.md Outdated
@shcheklein shcheklein temporarily deployed to dvc-landing-2807-use-pa-emgsrg December 11, 2019 20:58 Inactive
Copy link
Copy Markdown
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.

Shoot sorry, one last detail ^

```

`less` can be installed in other ways, just make sure it's available in
`cmd`/Powershell, where you run dvc. (This usually means adding the directory
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
`cmd`/Powershell, where you run dvc. (This usually means adding the directory
`cmd`/Powershell, where you run `dvc`. (This usually means adding the directory

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually I can commit this 🙂 One min...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah no, sorry. I don't have permission to push to your fork. Please image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(p.s. FYI you can allow upstream repo maintainers to push to the branch in the PR settings.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jorgeorpinel let's merge and fix it in-place? :) it's too minor to do a cycle.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

True, I can definitely do that!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed in 9955833.

@shcheklein shcheklein merged commit 917be5a into treeverse:master Dec 11, 2019
@jorgeorpinel
Copy link
Copy Markdown
Contributor

Thanks for the contribution and addressing all the feedback so promptly @xliiv !

jorgeorpinel added a commit that referenced this pull request Dec 11, 2019
@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 12, 2019

Thanks for the contribution and addressing all the feedback so promptly @xliiv !

thank you for your review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add info about pager on Windows

3 participants