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

Removed condescending language from Command Reference #1522

Closed
wants to merge 1 commit into from

Conversation

me-heer
Copy link

@me-heer me-heer commented Jul 3, 2020

Address #1394 (partially)

  • Reviewed and made changes to Command Reference.
  • I fixed a couple of typos and minimized simply removing all the occurrences of 'just'.

@jorgeorpinel

This comment has been minimized.

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.

Thanks for the small typo and grammar corrections 🙂

This PR is big... I only reviewed your first 6 changed files for now. Please try to apply my comments to all the other files I didn't review yet, where/if applicable.

Also, please copy the changes that you think are valid from https://github.com/iterative/dvc.org/pull/1396/files.

@@ -158,7 +158,7 @@ baseline-experiment <- First simple version of the model
bigrams-experiment <- Uses bigrams to improve the model
```

We can now just run `dvc checkout` that will update the most recent `model.pkl`,
We can now run `dvc checkout` that will update the most recent `model.pkl`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
We can now run `dvc checkout` that will update the most recent `model.pkl`,
We can now run `dvc checkout` to update the most recent `model.pkl`,

The paragraph will need formatting after this change though.

Same as in https://github.com/iterative/dvc.org/pull/1396/files#diff-d159337499c6716e1d89e1c12b5a8d09

Comment on lines -99 to +100
Let's employ a simple <abbr>workspace</abbr> with some data, code, ML models,
pipeline stages, such as the <abbr>DVC project</abbr> created for the
Let's employ a <abbr>workspace</abbr> with some data, code, ML models, pipeline
stages, such as the <abbr>DVC project</abbr> created for the
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 sure this is condescending but no strong opinion, we can change it. However, I believe this same paragraph exists in other refs. such as in checkout so let's apply the change consistently everywhere please.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mformihir what about the use of "just" in this doc? See https://github.com/iterative/dvc.org/pull/1396/files#diff-aac9973cd919dc8d6735e0de82ab7f62L47-R47 Do you think it's condescending?

Copy link
Contributor

Choose a reason for hiding this comment

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

no strong opinion, we can change it

On this, also. I'm not sure the phrase "Let's employ a workspace with some data..." reads right, without the adjective. Let's change the word "simple" for something more specific such as "new" or "fresh" please.

Comment on lines -123 to +124
Let's employ a simple <abbr>workspace</abbr> with some data, code, ML models,
pipeline stages, such as the <abbr>DVC project</abbr> created for the
Let's employ a <abbr>workspace</abbr> with some data, code, ML models, pipeline
stages, such as the <abbr>DVC project</abbr> created for the
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: new workspace; fresh workspace; clean workspace — something like that

@@ -256,8 +256,8 @@ $ dvc status -c
deleted: data/data.xml
```

One could do a simple `dvc fetch` to get all the data, but what if you only want
to retrieve the data up to our third stage, `train.dvc`? We can use the
One could do a `dvc fetch` to get all the data, but what if you only want to
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
One could do a `dvc fetch` to get all the data, but what if you only want to
We could use `dvc fetch` to get all the data, but what if you only want to

> (just download the file or directory).
> (just downloads the file or directory).
Copy link
Contributor

Choose a reason for hiding this comment

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

Good. But what about the other change from https://github.com/iterative/dvc.org/pull/1396/files#diff-06ee43874ed57c5e9a7512621fba5ed1L81-R81 ?

`remote://myremote/path/to/file` notation just means -> The `remote://myremote/path/to/file` notation means please

@jorgeorpinel
Copy link
Contributor

Hi @mformihir, will you be continuing this contribution? Thanks

@jorgeorpinel jorgeorpinel dismissed their stale review July 23, 2020 00:58

Looks like I'm going to have to finish this one...

@jorgeorpinel jorgeorpinel self-assigned this Jul 23, 2020
@shcheklein
Copy link
Member

Closing this as stale.

@shcheklein shcheklein closed this Aug 5, 2020
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.

3 participants