Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Conversation

@LawnGnome
Copy link
Contributor

@LawnGnome LawnGnome commented Nov 18, 2020

This roughs in some top level src-cli documentation, including:

  • Expanded src login documentation.
  • A description of the environment variables used by src-cli.
  • The Windows specific documentation previously only found in the src-cli repo.
  • Information on creating access tokens.
  • A bare bones command reference.

I'm on the fence about the utility of the reference: it's auto-generated from src-cli via an experimental doc command added in sourcegraph/src-cli#387, but we don't really have the structured data in src-cli right now to drive a good Markdown experience. Opinions welcome.

I've pushed this to https://docs.sourcegraph.com/@aharvey-login-docs/cli (alas, I forgot until after I'd opened this PR that docsite can't handle slashes in branch names, so I'll have to try to keep that up to date separate to this PR by double pushing).

This closes #14528 and closes #15882.

This roughs in some top level src-cli documentation, including an
autogenerated reference.
@LawnGnome LawnGnome added docs batch-changes Issues related to Batch Changes labels Nov 18, 2020
@LawnGnome LawnGnome requested a review from a team November 18, 2020 06:39
@sourcegraph-bot
Copy link
Contributor

sourcegraph-bot commented Nov 18, 2020

Notifying subscribers in CODENOTIFY files for diff f99c191...ccaaa2e.

Notify File(s)
@christinaforney doc/campaigns/quickstart.md
doc/cli/explanations/env.md
doc/cli/explanations/index.md
doc/cli/explanations/windows.md
doc/cli/how-tos/creating_an_access_token.md
doc/cli/how-tos/index.md
doc/cli/index.md
doc/cli/quickstart.md
doc/cli/references/api.md
doc/cli/references/campaigns/apply.md
doc/cli/references/campaigns/index.md
doc/cli/references/campaigns/new.md
doc/cli/references/campaigns/preview.md
doc/cli/references/campaigns/repositories.md
doc/cli/references/campaigns/validate.md
doc/cli/references/config/edit.md
doc/cli/references/config/get.md
doc/cli/references/config/index.md
doc/cli/references/config/list.md
doc/cli/references/doc.go
doc/cli/references/extensions/copy.md
doc/cli/references/extensions/delete.md
doc/cli/references/extensions/get.md
doc/cli/references/extensions/index.md
doc/cli/references/extensions/list.md
doc/cli/references/extensions/publish.md
doc/cli/references/extsvc/edit.md
doc/cli/references/extsvc/index.md
doc/cli/references/extsvc/list.md
doc/cli/references/gen.go
doc/cli/references/index.md
doc/cli/references/login.md
doc/cli/references/lsif/index.md
doc/cli/references/lsif/upload.md
doc/cli/references/orgs/create.md
doc/cli/references/orgs/delete.md
doc/cli/references/orgs/get.md
doc/cli/references/orgs/index.md
doc/cli/references/orgs/list.md
doc/cli/references/orgs/members/add.md
doc/cli/references/orgs/members/index.md
doc/cli/references/orgs/members/remove.md
doc/cli/references/repos/delete.md
doc/cli/references/repos/disable.md
doc/cli/references/repos/enable.md
doc/cli/references/repos/get.md
doc/cli/references/repos/index.md
doc/cli/references/repos/list.md
doc/cli/references/search.md
doc/cli/references/serve-git.md
doc/cli/references/users/create.md
doc/cli/references/users/delete.md
doc/cli/references/users/get.md
doc/cli/references/users/index.md
doc/cli/references/users/list.md
doc/cli/references/users/tag.md
doc/cli/references/validate.md
doc/cli/references/version.md
doc/sidebar.md

Copy link
Contributor

@mrnugget mrnugget 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 really cool! But I think this should be its own top-level directory in the docs folder and the sidebar.

Why?

  • I never would've looked for CLI documentation under "integrations"
  • Once you're a few levels deep into the CLI docs the subsections are not highlighted anymore.

Edit: regarding the reference: I'm not 100% convinced whether auto-generated docs are great, but I think this is a great first step.

@LawnGnome
Copy link
Contributor Author

This is really cool! But I think this should be its own top-level directory in the docs folder and the sidebar.

Why?

* I never would've looked for CLI documentation under "integrations"

* Once you're a few levels deep into the CLI docs the subsections are not highlighted anymore.

I agree with you, but I was a little nervous adding a brand new top level section for the CLI: it's important to us, but I'm not sure it's that important globally yet for Sourcegraph.

@christinaforney, what do you think?

@mrnugget
Copy link
Contributor

I agree with you, but I was a little nervous adding a brand new top level section for the CLI: it's important to us, but I'm not sure it's that important globally yet for Sourcegraph.

I think it is important, especially since having it top-level would make it easier to discover other things we want to document in the future: versioning, releases, how to run src-cli in Docker, etc. These all fit nicely under explanations/how-tos/reference and it'd a shame if they are hidden in a sub-directory.

@christinaforney
Copy link
Contributor

I agree with @mrnugget - We can put this in the top level nav. I might end up doing some changing the left nav later, but the docs path should be doc/cli, and we can add it between 'Extensions' and 'Adopting' in the left nav.

return errors.Wrap(err, "setting up go.mod")
}

goGet := exec.Command("go", "get", "github.com/sourcegraph/src-cli/cmd/src@581f9f532d890e6c2dc2ec2a130ab0d3d857e715")
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 need to remove the explicit commit hash here before merging, but this is required right now to pull in sourcegraph/src-cli#387.

@LawnGnome
Copy link
Contributor Author

This now has reference generation via go generate (oh no), and the new top level docs are at https://docs.sourcegraph.com/@aharvey-login-docs/cli.

We might want to exclude this from the default ./dev/generate.sh flow, since this is irrelevant 99% of the time, but let's see how bad this is first.

Copy link
Member

@eseliger eseliger left a comment

Choose a reason for hiding this comment

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

That's hands-down the best bang for buck we could have gotten for this, great work!

@eseliger
Copy link
Member

Ah found one potential ever-changing value, especially in CI:

image

@LawnGnome
Copy link
Contributor Author

Ah found one potential ever-changing value, especially in CI:

Ah, dang. I do have a horrifying idea on how to fix that, though. One moment.

@LawnGnome
Copy link
Contributor Author

@eseliger Updated in conjunction with sourcegraph/src-cli@f48121d.

@eseliger
Copy link
Member

Nice little hack, thanks for the prompt fix!

Copy link
Contributor

@mrnugget mrnugget 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!

We might want to exclude this from the default ./dev/generate.sh flow, since this is irrelevant 99% of the time, but let's see how bad this is first.

I agree with the "we might want" and think it doesn't need to be patched into ./dev/generate.sh in this PR. Feel free to add that in a follow-up though. (We could, for now, mention it in src-cli/DEVELOPMENT.md so that it's run when bumping the MinimumVersion)


### Install manually

1. Download the latest [src_windows_amd64.exe](https://sourcegraph.com/.api/src-cli/src_windows_amd64.exe)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this also mention that we prefer you use the version recommended by your Sourcegraph instance?

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 chose this intentionally: using the sourcegraph.com version means that we can provide a straight up link here, rather than requiring the user to construct their own URL based on their host name. Essentially, I want to optimise for time to first src command.

The catch here is obvious: if we make a change in a later src-cli version that breaks backward compatibility with older Sourcegraph versions, then this advice becomes dangerous, and probably turns into a support ticket.

This is getting into Versioning RFC fodder (for when I eventually have time to write it), but I'd prefer to move us explicitly to a model where the latest src-cli is always* safe for use with any reasonable Sourcegraph version, rather than users having to figure out what the right version for their Sourcegraph is.

I'm not opposed to adding a note about getting the recommended version for your Sourcegraph instance from some link, but I'd prefer to keep the default instruction to download the most recent version for simplicity. Another option would be to add a proper download page to Sourcegraph: we could link it from the footer, and do some autodetection based on the user agent to suggest the most likely binary — that would be easier to document, I think, that a DIY URL.

I'm going to go ahead and merge this PR (once I've addressed the other remaining feedback), since the release is tomorrow, but I hear you, don't consider this conversation closed, and we should iterate more on this. 😄

* Yes, that's a big word, and I'm a little scared of it. 😄

@LawnGnome
Copy link
Contributor Author

I agree with the "we might want" and think it doesn't need to be patched into ./dev/generate.sh in this PR. Feel free to add that in a follow-up though. (We could, for now, mention it in src-cli/DEVELOPMENT.md so that it's run when bumping the MinimumVersion)

Yep, that's what I've done in sourcegraph/src-cli#391.

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

Labels

batch-changes Issues related to Batch Changes docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doc: replicate src campaign apply -help as a reference page cli: better document login process and environment variables

6 participants