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 spec.homepage field to the plugin manifest #158

Merged
merged 4 commits into from
Mar 8, 2019

Conversation

superbrothers
Copy link
Contributor

This PR adds spec.homepage field to the plugin manifest.

/close #157


The spec.homepage field of the plugin manifest defines the url of the
plugin project homepage. The field value should be a valid url.

In addition, if the spec.homepage field is defined in plugin manifest,
kubectl krew info command displays that field value as HOMEPAGE as follows:

$ kubectl krew info krew
NAME: krew
URI: https://storage.googleapis.com/krew/v0.2.1/krew.tar.gz
SHA256: dc2f2e1ec8a0acb6f3e23580d4a8b38c44823e948c40342e13ff6e8e12edb15a
VERSION: v0.2.1
HOMEPAGE: https://github.com/GoogleContainerTools/krew
...

The `spec.homepage` field of the plugin manifest defines the url of the
plugin project homepage. The field value should be a valid url.

In addition, if the `spec.homepage` field is defined in plugin manifest,
`kubectl krew info` command displays that field value as HOMEPAGE as follows:

    $ kubectl krew info krew
    NAME: krew
    URI: https://storage.googleapis.com/krew/v0.2.1/krew.tar.gz
    SHA256: dc2f2e1ec8a0acb6f3e23580d4a8b38c44823e948c40342e13ff6e8e12edb15a
    VERSION: v0.2.1
    HOMEPAGE: https://github.com/GoogleContainerTools/krew
    ...
@codecov-io
Copy link

codecov-io commented Mar 7, 2019

Codecov Report

Merging #158 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #158   +/-   ##
=======================================
  Coverage   50.91%   50.91%           
=======================================
  Files          13       13           
  Lines         711      711           
=======================================
  Hits          362      362           
  Misses        297      297           
  Partials       52       52

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c94ea89...32c6d76. Read the comment docs.

Copy link
Member

@ahmetb ahmetb left a comment

Choose a reason for hiding this comment

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

can you also edit testdata/? there must be some complete manifests there. we wanna make sure we parse them correctly.

pkg/index/validate.go Outdated Show resolved Hide resolved
cmd/krew/cmd/info.go Show resolved Hide resolved
By this change, the output fields of `krew info` command are reordered as follows:
```
$ out/bin/krew-darwin_amd64 info krew
NAME: krew
URI: https://storage.googleapis.com/krew/v0.2.1/krew.tar.gz
SHA256: dc2f2e1ec8a0acb6f3e23580d4a8b38c44823e948c40342e13ff6e8e12edb15a
VERSION: v0.2.1
HOMEPAGE: https://github.com/GoogleContainerTools/krew
CAVEATS:
\
...
```
pkg/index/validate_test.go Outdated Show resolved Hide resolved
@superbrothers
Copy link
Contributor Author

Should I squash this PR?

Copy link
Member

@ahmetb ahmetb left a comment

Choose a reason for hiding this comment

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

Thanks LGTM.
We can add the validation later if needed.

@ahmetb
Copy link
Member

ahmetb commented Mar 8, 2019

Also home page is part of documentation technically. It's not good to fail parsing the index during an upgrade because some homepage URL was invalid. I think this is better.

@ahmetb ahmetb merged commit c931212 into kubernetes-sigs:master Mar 8, 2019
@superbrothers superbrothers deleted the add-homepage-field branch March 8, 2019 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal: Add Homepage, License and Author fields to Plugin
4 participants