Add validation for screenshots and icons #537
Conversation
| @@ -78,7 +78,10 @@ type Version struct { | |||
| } | |||
|
|
|||
| type Image struct { | |||
| Src string `config:"src" json:"src" validate:"required"` | |||
| // Src is relative inside the package | |||
There was a problem hiding this comment.
@mtojek Any ideas for better naming here?
| // Src is relative inside the package | ||
| Src string `config:"src" json:"src" validate:"required"` | ||
| // Path is the absolute path in the url | ||
| Path string `config:"path" json:"path"` |
There was a problem hiding this comment.
@jfsiii If we go forward, this would be a breaking change on the Kibana side. Also ideas around "correct" naming?
There was a problem hiding this comment.
Perhaps pathname vs path as in url.pathname https://developer.mozilla.org/en-US/docs/Web/API/URL/pathname & https://nodejs.org/docs/latest-v12.x/api/url.html#url_url_pathname
There was a problem hiding this comment.
path is pretty much in line with the other places we use it like for downloads: https://github.com/elastic/package-registry/blob/master/docs/api/package.json#L10 If we adjust it, we should adjust it everywhere.
Perhaps we can rename src to local_path, relative_path or something like this?
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
|
|
I'm unclear why this new value is stored in the registry. Do we need to support both forms? Can we just pick one? If we need both do we need to keep |
|
@jfsiii We need one, that is used for the dev building the package (src) and the other one is that we expose on the registry side to provide the full path (path). Both show up in the output as by default we just convert the manifest.yml to json, so the src also shows up. We could hide it if we want in the json output. We are not tied to any names at the moment, happy to change. |
|
@ycombinator Moving forward, I guess this is something that should be validated on the package-spec level? |
Yes, |
ruflin
force-pushed
the
icon-screenshot-validation
branch
from
8b79947 to
6bf04b3
Compare
This validates all the defined icons and screenshots and errors out, if a src is not found. Some testing packages needed updating because of this.
ruflin
force-pushed
the
icon-screenshot-validation
branch
from
dc67460 to
2b16328
Compare
|
@ycombinator I made this ready for review as it is just a small change, code was already around and I don't think we have this yet in elastic-package. As soon as we get it into elastic-package and it is run for all places where we have packages, we should get rid of it. |
This validates all the defined icons and screenshots and errors out, if a src is not found. Some testing packages needed updating because of this.