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

Add documentation for default index URI variable #669

Merged
merged 2 commits into from
Dec 5, 2020

Conversation

chriskim06
Copy link
Member

So I wasn't super sure where this should go. After looking around I was thinking about either adding it as a separate page under the setup/ tree or consolidating this with the update check doc. I landed on consolidating the two but let me know what you guys think.

Fixes #637

/assign @ahmetb
/assign @corneliusweig

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 28, 2020
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 28, 2020
@ahmetb
Copy link
Member

ahmetb commented Nov 28, 2020

I think this should go under User Guide directly, right after "Using Custom Plugin Indexes" which is currently the last page in there.

We can call this "Advanced Configuration" to indicate most users should not be bothered by this page while reading the user guide.


### Use a different default index

By default Krew uses [krew-index][ki] as the default plugin index. This is the
Copy link
Member

Choose a reason for hiding this comment

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

I'd rewrite this paragraph as follows:

When Krew is installed, it automatically initializes an index named default pointing to [krew-index][ki] repository. If you choose to remove this index, see [custom indexes] documentation. You can also force Krew to point it to another repository by setting KREW_DEFAULT_INDEX_URI.

+mash that up with existing links + next paragraph you have.

Copy link
Member Author

Choose a reason for hiding this comment

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

What about:
 

When Krew is installed, it automatically initializes an index named default
pointing to the [krew-index][ki] repository. You can force Krew to use a
different repository by setting KREW_DEFAULT_INDEX_URI before running the
[installation instructions]({{<ref "install.md">}}) or after [removing the
default index]({{<ref "using-custom-indexes.md">}}).

To use a different default index, set the KREW_DEFAULT_INDEX_URI environment
variable in your ~/.bashrc, ~/.bash_profile, or ~/.zshrc:

I think it would make sense to keep the parts on when the env var needs to be set together instead of mentioning that you can set it after deleting the default index in the first paragraph and before krew installation in the second paragraph.

Also should I make this draft: true? IIRC the docs get published automatically.

Copy link
Member

Choose a reason for hiding this comment

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

Don't worry about setting it as draft without a release being available. I highly suspect anyone’ll come looking for it.

I think your edit is good, make sure to mention this URI needs to point to a git repository reference (can be a real url, or another git-remote protocol like foo://bar/quux.git).

Copy link
Member Author

Choose a reason for hiding this comment

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

Made the change, @ahmetb mind taking another look?

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 30, 2020
Copy link
Contributor

@corneliusweig corneliusweig left a comment

Choose a reason for hiding this comment

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

/lgtm
Thanks for giving our docs some ❤️

@@ -0,0 +1,38 @@
---
title: Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe talk about customization?

export KREW_NO_UPGRADE_CHECK=1
```

### Use a different default index
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
### Use a different default index
### Use a custom default index

### Use a different default index

By default Krew uses [krew-index][ki] as the default plugin index. This is the
repository that is normally used for plugin discovery. However, you can use a
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
repository that is normally used for plugin discovery. However, you can use a
This index contains a curated list of plugins. However, you can use a

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 5, 2020
@chriskim06
Copy link
Member Author

@corneliusweig I think your review comments were on an older version of this PR

@corneliusweig
Copy link
Contributor

Yes indeed. Epic fail with GitHub app for mobile :-S
Looks like I also need to
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chriskim06, corneliusweig

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 5, 2020
@k8s-ci-robot k8s-ci-robot merged commit c5d05f3 into kubernetes-sigs:master Dec 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow overriding DefaultIndexURI from an env variable
4 participants