-
Notifications
You must be signed in to change notification settings - Fork 371
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
site: custom index docs #618
site: custom index docs #618
Conversation
I'll make a pass by adding commits, stay tuned. |
Just did a pass by:
|
Actually could not push the commit. @chriskim06 do you mind choosing "allow maintainers to push additional commits" on the right? |
Signed-off-by: Ahmet Alp Balkan <[email protected]>
Pushed some more changes to https://deploy-preview-618--kubernetes-sigs-krew.netlify.app/docs/developer-guide/custom-indexes/ |
Signed-off-by: Ahmet Alp Balkan <[email protected]>
/lgtm |
/lgtm cancel we need to add |
Is that setting external to this PR? |
No, you'll need to update the .md files you added and add the property to their header section. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reads pretty good. Just small nits from my side.
## Duplicate plugin names | ||
|
||
Your custom index can contain plugins that have the same name as the ones in | ||
`krew-index`. | ||
|
||
Users of your index will need to install your plugin using the | ||
explicit `<INDEX>/<PLUGIN>` syntax. See the [user guide][ug]. | ||
|
||
[ug]: {{< relref "../user-guide/using-custom-indexes.md">}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has considerable overlap with using-custom-indexes
, and duplicates some information. Do we really need it?
(also the heading does not fit to the second paragraph)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok with whatever. I think it might be good to have it repeated here as a consideration for people writing plugins for custom indexes and to let them know that people will need to install their plugin differently.
However, some plugin authors may choose to host their own indexes that contain | ||
their curation of kubectl plugins. These are called "custom plugin indexes". | ||
|
||
## Adding a custom index |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the title isn’t good.
It would be better to explain in this section “The default index” and
- how it can be removed
- or pointed to another repo
- and how plugins without an explicit index name mean “default”
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the adding/removing/listing indexes sections are important to showcase the new index command right? I agree though that a section explaining the default index would be good, I can move the advanced usage section I had and change it to reflect the points you listed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think this should work well. You could also explain how plugin names are resolved. Iow, that a default/
prefix is added implicitly, if no index name is given. Then you could also explain how plugin name clashes are handled and remove the Duplicate plugin names
section in the other page.
foo https://github.com/foo/custom-index.git{{</output>}} | ||
``` | ||
|
||
## The default index |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we perhaps use code block notation for all “default”s in this paragraph?
|
||
## The default index | ||
|
||
When a plugin doesn't have an explicit `INDEX_NAME` prefix it refers to a plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
, it
with the same name across different indexes. | ||
|
||
Krew ships with [`krew-index`][ki] as the default index, but this can be removed | ||
by passing `default` to the remove command. Once this is removed, you can add |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...Removed using the “kubectl krew index remove default” command
foo https://github.com/foo/custom-index.git{{</output>}} | ||
``` | ||
|
||
## The default index |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section should ideally come after installing plugins from custom indexes (keep in mind most people might not care about the default index nuances)
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmetb, chriskim06 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 |
/hold cancel |
Let me know what you guys think. Also what do I need to do to mark this as draft?
Related issue: #566
/area multi-index
/area docs