Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

documentation improvements for new users #819

Merged
merged 10 commits into from
Jul 21, 2017

Conversation

afeld
Copy link
Contributor

@afeld afeld commented Jul 15, 2017

Hello from Gophercon! I'm a new Dep user/contributor, so decided to work on the first-run experience in terms of documentation.

What does this do / why do we need it?

See the commit messages for details (I tried to keep each one small/discrete), but the high-level overview is:

  • Moved the FAQ under the docs/ folder, to have one less file at the top level
    • This could probably be done with other files, but I didn't want to get too carried away.
  • Split and expand the Usage information into initial setup and day-to-day interactions with Dep
  • Don't encourage adding/changing of dependencies through the CLI directly
  • Minor formatting fixes

What should your reviewer look out for in this PR?

Whether the updated instructions are still recommended/accurate.

Do you need help or clarification on anything?

I don't think so, but I don't know what I don't know.

Which issue(s) does this PR fix?

None.

@googlebot
Copy link
Collaborator

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
  • In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

@afeld
Copy link
Contributor Author

afeld commented Jul 15, 2017

I'm a federal employee, so getting the CLA signed may not be quick 😕 Working it up the chain, but would love feedback in the meantime.

README.md Outdated
Get the tool via

```sh
$ go get -u github.com/golang/dep/cmd/dep
```

Typical usage on a new repo might be
To set up Dep on a project, run the following from your project root directory:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps this is an over-simplification? /cc #731

Copy link
Collaborator

Choose a reason for hiding this comment

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

We use dep instead of "Dep".

Copy link
Collaborator

Choose a reason for hiding this comment

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

"To start managing dependencies using dep, ..."?

I don't know, but I still think we can come up with something better here. 😁

README.md Outdated

```sh
$ dep init
$ dep ensure -update
```

If your project was already using a `vendor/` directory, it was backed up to `_vendor-TIMESTAMP/`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"If a vendor/ directory existed in your project root, it should have been backed up to _vendor-TIMESTAMP/."?

I'm not sure, but I think this one could use some rephrasing.

@@ -24,7 +24,6 @@ Summarize the question and quote the reply, linking back to the original comment
* [Why did `dep` use a different revision for package X instead of the revision in the lock file?](#why-did-dep-use-a-different-revision-for-package-x-instead-of-the-revision-in-the-lock-file)
* [Why is `dep` slow?](#why-is-dep-slow)
* [How does `dep` handle symbolic links?](#how-does-dep-handle-symbolic-links)
* [`dep` deleted my files in the vendor directory!](#dep-deleted-my-files-in-the-vendor-directory)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you remove this question?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the section it links to, because that information is now in the README.

README.md Outdated
Get the tool via

```sh
$ go get -u github.com/golang/dep/cmd/dep
```

Typical usage on a new repo might be
To set up Dep on a project, run the following from your project root directory:
Copy link
Collaborator

Choose a reason for hiding this comment

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

We use dep instead of "Dep".

README.md Outdated
Get the tool via

```sh
$ go get -u github.com/golang/dep/cmd/dep
```

Typical usage on a new repo might be
To set up Dep on a project, run the following from your project root directory:
Copy link
Collaborator

Choose a reason for hiding this comment

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

"To start managing dependencies using dep, ..."?

I don't know, but I still think we can come up with something better here. 😁

@afeld
Copy link
Contributor Author

afeld commented Jul 15, 2017

@carolynvs Went ahead and added some of the things we talked about in person. Thanks for the help!

Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

these are generally fantastic changes - really feel like they're taking the docs in the right direction!

just a few notes. hopefully you'll get through that CLA soon 😄

README.md Outdated
## Usage

There is one main subcommand you will use: `dep ensure`. `ensure` synchronizes
your dependencies in `vendor/` to make sure they match what's in your `import`s
Copy link
Member

Choose a reason for hiding this comment

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

Close, but a little off the mark. Maybe:

ensure first makes sure Gopkg.lock is consistent with your imports and Gopkg.toml, and then that vendor has is poopulated with exactly what's described in Gopkg.lock.

README.md Outdated
(if your `vendor/` directory isn't [checked in with your code](](docs/FAQ.md#should-i-commit-my-vendor-directory)))

```sh
$ dep ensure
Copy link
Member

Choose a reason for hiding this comment

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

can we stick TODOs in these files without them showing up in normal reading mode? If so, we should link to #489, as it may change what we tell people here a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Might want to add a TODO in that pull request, as well.


1. Remove from `Gopkg.toml`, if it was in there.

### Testing changes to a dependency
Copy link
Member

Choose a reason for hiding this comment

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

This simple advice will work for quickly testing local changes, but not for actually keeping any of them in Gopkg.lock or vendor in any kind of portable way. It's probably worth noting that, as well as that we may not have a really good solution for this until after toolchain integration (see gist)

It may be worth linking to virtualgo, as well, as it offers one interim attempt at a solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@sdboyer
Copy link
Member

sdboyer commented Jul 18, 2017

@afeld oh, also - there's no way for you to sign the CLA as an individual?

Copy link
Collaborator

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

I ❤️ IT! One million pony points for giving the doc, and therefore our users, some much needed love.

README.md Outdated

Get the tool via

```sh
$ go get -u github.com/golang/dep/cmd/dep
```

Typical usage on a new repo might be
To start managing dependencies using dep, run the following from your project root directory:

```sh
$ dep init
$ dep ensure -update
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that the instructions above have changed from "here are typical commands" to "Do this to start using dep", we should remove dep ensure -update.

All you need to start using dep is init, and if they had specific versions identified by dep either from their GOPATH, or other dependency managers, we don't want to immediately clobber them. 😀

README.md Outdated

```sh
$ dep init
$ dep ensure -update
```

To update a dependency to a new version, you might run
`dep init` will do the following:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggestion: change to "This does the following".

README.md Outdated
### Adding a dependency

1. Add the `import` in your source code file(s).
1. Download via dep.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe change this to

Run the following command to update your Gopkg.lock and populate `vendor/` with the new dependency.


### Checking the status of dependencies

```sh
Copy link
Collaborator

Choose a reason for hiding this comment

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

status will also tell you when your lock file is out of date, which can happen when you've added imports, or edited the manifest, but have not yet rerun dep ensure. Let's show an example of that output and let people know the way to "fix" it is to run dep ensure again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mind if we take care of that as a follow-up?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup! That's fine.

README.md Outdated
dependency into `vendor/` based on your manifest, as if you were installing from
scratch.

To test out code that is pushed, see [changing dependencies](#changing-dependencies).
Copy link
Collaborator

Choose a reason for hiding this comment

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

"To test out code that has been pushed to a new version, branch or fork"

Copy link
Collaborator

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

🎉

@afeld
Copy link
Contributor Author

afeld commented Jul 19, 2017

Realizing that, while I was at Gophercon in my official capacity, I made these contributions as myself. Signed the contributor agreement as an individual, and rebased the commits to use my personal instead of work email. Hopefully we'll get the CLA signed as an organization so contributions can happen more seamlessly in the future. Good to go!

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

Copy link
Collaborator

@ibrasho ibrasho left a comment

Choose a reason for hiding this comment

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

Minor nits. No showstoppers though. 👍


Get the tool via

```sh
$ go get -u github.com/golang/dep/cmd/dep
```

Typical usage on a new repo might be
To start managing dependencies using dep, run the following from your project root directory:
Copy link
Collaborator

@ibrasho ibrasho Jul 19, 2017

Choose a reason for hiding this comment

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

dep with backticks. 😁

This does the following:

1. Look for [existing dependency management files](docs/FAQ.md#what-external-tools-are-supported) to convert
1. Check if your dependencies use dep
Copy link
Collaborator

@ibrasho ibrasho Jul 19, 2017

Choose a reason for hiding this comment

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

dep with backticks.

Copy link
Collaborator

@carolynvs carolynvs Jul 19, 2017

Choose a reason for hiding this comment

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

I don't want to sidetrack/bikeshed this issue (so we can continue discussing in a new issue if necessary), but would ask that we don't backtick every mention of dep. 😀

One easy way to think about it this: only use backticks if we are telling them to run a command, referring to a dep command, etc. Anytime we are simply referring to the dep project, leave off the special formatting.

Examples:

  • Check if your dependencies use dep...
  • Run dep ensure to populate the vendor directory...
  • We recommend using dep in your Makefiles...
  • Verify that dep is on your $PATH...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would argue that we are almost always referring to dep as the binary (or CLI tool) in the docs. But I'm okay with that anyway. 😁

README.md Outdated

### Adding a dependency

1. Add the `import` in your source code file(s).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe: "import the package in your *.go source code file(s)."?


### Testing changes to a dependency

Making changes in your `vendor/` directory directly is not recommended, as dep
Copy link
Collaborator

@ibrasho ibrasho Jul 19, 2017

Choose a reason for hiding this comment

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

dep with backticks. 😁

README.md Outdated
dependency into `vendor/` based on your manifest, as if you were installing from
scratch.

This solution works for short-term use, but for something longer-term, take a
Copy link
Collaborator

Choose a reason for hiding this comment

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

"long-term"

@carolynvs
Copy link
Collaborator

@afeld I would love to merge this but some recent commits conflict with this PR. Let me know if you'd like to resolve them yourself, or I can do that for you. Happy to help either way.

The top-level directory is getting a bit cluttered, so group the FAQ
alongside the other documentation. Also did some minor formatting
cleanup of that file.
- Clarified that the Usage commands in the README apply to both new and
  existing projects
- Move information about the vendor directory backup into the README.
  This gives the user the necessary context as soon as they would need
  it, rather than needing to stumble upon it (somewhat buried) in the FAQ.
- Explain that `dep init` will try and import from other package
  managers
- Explain the day-to-day workflow of using Dep.
The former seems to be falling out of favor, and may confuse users about
which is preferable to use.
Trying to match the different common use cases a user will have for interacting
with dep.
…o dependencies

...to the README. Also, expanded on the description of what `dep init` does. All of this
was taken from @carolynvs's lightning talk on dep.

http://carolynvs.com/dep-in-10
Switched to `Setup` and `Usage` instead of `Initial setup` and
`Day-to-day workflows`. Also made both of those top-level section
headings, so that the various workflows aren't nested so deeply.
My hypothesis is that these explanations will be more useful as users
are reading about the sections themselves, rather than low down in the
FAQ, where they could easily be missed.
@afeld
Copy link
Contributor Author

afeld commented Jul 21, 2017

Rebased, and took care of the new suggestions.

README.md Outdated

```sh
$ dep ensure github.com/pkg/errors@^0.8.0
$ dep ensure -update
Copy link
Collaborator

Choose a reason for hiding this comment

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

One more follow-up issue (I'll create after this is merged) is to cover how to update a single package, and how to pick the version/revision.

README.md Outdated

There is one main subcommand you will use: `dep ensure`. `ensure` first makes
sure `Gopkg.lock` is consistent with your `import`s and `Gopkg.toml`, and then
that `vendor/` is populated with exactly what's described in `Gopkg.lock`. `dep
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry to add something at the last minute but someone was just tripped up by this definition of dep ensure yesterday. #865 (comment)

`ensure` first makes sure `Gopkg.lock` is consistent with your `import`s and `Gopkg.toml`. Then if any changes are detected,  populates `vendor/` with exactly what's described in `Gopkg.lock`.

We don't validate that the vendor directory matches the lock currently, and hopefully this will make that more clear! 😀

@carolynvs
Copy link
Collaborator

YOU ARE AWESOME! 🎉 Thanks for the much needed documentation.

@carolynvs carolynvs merged commit bcfa7ee into golang:master Jul 21, 2017
@carolynvs
Copy link
Collaborator

carolynvs commented Jul 21, 2017

I've gone ahead and added follow-up issues #876 and #877.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants