fix: refresh getting started documentation #80
Conversation
Signed-off-by: m.nabokikh <[email protected]>
Co-authored-by: Nate W. <[email protected]> Signed-off-by: m.nabokikh <[email protected]>
nabokihms
force-pushed
the
refresh-getting-started-doc
branch
from
009d585 to
cd75fae
Compare
|
Thanks for updating the docs @nabokihms! I've made one suggestion in-line. I'm also fairly new to Dex, so I'm not sure, but I don't think this PR quite fixes #78. Specifically:
I'm not seeing any additional explanation of how to use Docker in this update. I'm not suggesting we don't merge this change, I'm just suggesting that it not close #78 just yet. |
|
Yeah, we may leave #78 open. I have changed the top message description. The document about how to pull docker images will be added in another PR. |
|
Here is what I had in mind for this: compiling binaries should be moved to the development section. Users should not compile binaries IMHO. (see #71) The getting started guide should basically be a docker-compose file somewhere with an example config. Either in a new repo, or just added to the documentation, or in the dex repo in a subdir, not sure. |
|
Great roadmap! We have the docker-compose file to run storage backends. Maybe we can increase its functionality and add some examples... Anyway, yes, it's better to use docker for a quick start. About this PR. I consider this one as a quick fix to avoid some awkward situations. It is something that we can apply right now and something that can bring benefits for further doc development and users. |
| @@ -9,17 +9,21 @@ weight: 10 | |||
|
|
|||
| ## Building the dex binary | |||
|
|
|||
| Dex requires a Go installation and a GOPATH configured. For setting up a Go workspace, refer to the [official documentation][go-setup]. Clone it down the correct place, and simply type `make` to compile the dex binary. | |||
There was a problem hiding this comment.
We may not want to remove the reference to the GOPATH here as we do reference it later on. (and I don't think it is setup by default if you install Go)
There was a problem hiding this comment.
Gopath is not mandatory after migrating to go modules. If it is confusing, I can completely remove it.
There was a problem hiding this comment.
I'm able to run the examples w/o setting the $GOPATH, and if it's not mandatory, it may be best to remove it as it is extra detail that makes the getting started more complicated.
There was a problem hiding this comment.
Ok, I have no objections.
content/docs/getting-started.md
Outdated
| $ make | ||
| ``` | ||
| > **Note:** It is possible to clone the repository outside $GOPATH |
There was a problem hiding this comment.
Noob question, and the answer may or may not need to be brought in if it's obvious to more seasoned dex devs, but what's the advantage to cloning the repo in the GOPATH?
There was a problem hiding this comment.
I see some benefits, but they are minor.
- $GOPATH is probably exists
- If you want to build an old version of a dex, you need to move the repository to $GOPATH
There was a problem hiding this comment.
I'd like to suggest then that we do remove the $GOPATH references, but potentially that we move them to an 'advanced' setup page or something like it? @sagikazarmark thoughts?
|
I'm not a technical reviewer on this project, but I went through and tested the examples provided (added some comments), but it works so: Possibly a comment for another issue: we may want to make it more obvious in the "Running a client" section that the dex needs to be run with the example config noted in the previous section. |
Signed-off-by: m.nabokikh <[email protected]>
This PR fixes some issues with "Getting Started" documentation (that were mentioned in #78).