Add & validate plugin meta for plugin viewer #79
Conversation
|
@ibuziuk @garagatyi FYI |
| CATEGORIES=("Editor" "Debugger" "Formatter" "Language" "Linter" "Snippet" "Theme" "Other") | ||
|
|
||
| function check_category() { | ||
| for CATEGORY in "${CATEGORIES[@]}" |
There was a problem hiding this comment.
Indentation is not consistent. Please align it
| return 1 | ||
| } | ||
|
|
||
| cd plugins || exit 1 |
There was a problem hiding this comment.
why not set exit on error? This would make the script more understandable.
There was a problem hiding this comment.
thx, forgot about that one
| # or else it is invalid | ||
| if [[ "${FIELD}" == "category" ]];then | ||
| yq w meta.yaml category "Other" -i | ||
| VALUE=$(yq r meta.yaml "$FIELD") |
There was a problem hiding this comment.
Reading this value here is not needed - it was read before.
| if [[ "${FIELD}" == "category" ]];then | ||
| yq w meta.yaml category "Other" -i | ||
| VALUE=$(yq r meta.yaml "$FIELD") | ||
| if ! check_category "${VALUE}";then |
There was a problem hiding this comment.
Usage of 6 level-deep nesting makes code too complicated for maintenance. Please, refactor.
set_plugin_dates.sh
Outdated
| # | ||
| # SPDX-License-Identifier: EPL-2.0 | ||
| # | ||
| cd plugins || exit 1 |
There was a problem hiding this comment.
It is better to set exit on error mode
set_plugin_dates.sh
Outdated
| cd "${VERSION_DIR_NAME}" || exit 1 | ||
|
|
||
| DATE=$(date -I) | ||
| yq w meta.yaml firstPublicationDate "${DATE}" -i || exit 1 |
There was a problem hiding this comment.
Can you elaborate what this command does? I'm not familiar with the syntax of yq and it seems that we overwrite dates on each build of the image.
There was a problem hiding this comment.
yes, as of now, these dates will be generated at built of docker image, kind of a placeholder
There was a problem hiding this comment.
We can check if the field is missing then add it. It would pretty accurate. But would work for the first publication date only.
|
@garagatyi please review again, I've addressed the comments |
set_plugin_dates.sh
Outdated
| cd "${VERSION_DIR_NAME}" || exit 1 | ||
|
|
||
| DATE=$(date -I) | ||
| yq w meta.yaml firstPublicationDate "${DATE}" -i || exit 1 |
There was a problem hiding this comment.
We can check if the field is missing then add it. It would pretty accurate. But would work for the first publication date only.
| @@ -6,3 +6,6 @@ title: Eclipse Theia for Eclipse Che | |||
| description: Eclipse Theia | |||
| icon: https://raw.githubusercontent.com/theia-ide/theia/master/logo/theia-logo-no-text-black.svg?sanitize=true | |||
| url: https://github.com/ws-skeleton/che-editor-theia/releases/download/untagged-cf91340b0b4af08ec756/che-editor-plugin.tar.gz | |||
| publisher: Red Hat, Inc. | |||
| category: Editor | |||
| repository: https://github.com/ws-skeleton/che-editor-theia/ | |||
There was a problem hiding this comment.
Location of the repo is about to change. I suggest changing it right away not to need to update later. See eclipse-che/che-theia#48
| return 0 | ||
| } | ||
|
|
||
| function check_category() { |
There was a problem hiding this comment.
Could you add comments (aka docs) that describe function parameters and what it actually does? FYI it is unusual that function called check changes files.
There was a problem hiding this comment.
added some comments to functions
| @@ -9,3 +9,7 @@ attributes: | |||
| extension: "vscode:extension/SonarSource.sonarlint-vscode" | |||
| container-image: "garagatyi/remotetheia:java" | |||
| containerImage: "garagatyi/remotetheia:java" | |||
| firstPublicationDate: "2019-02-05" | |||
| category: Other | |||
|
@skabashnyuk @mkuznyetsov FYI prod-preview has been updated to 6.18.0 [1] I believe this PR can be merged now |
Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>
mkuznyetsov
force-pushed
the
newfields
branch
from
1744e69
to
b59a745
Compare
Signed-off-by: Mykhailo Kuznietsov mkuznets@redhat.com
What does this PR do?
Adding more fields for plugin meta YAML, for use by plugin-viewer
Added scripts for validation of mandatory fields, that are run during Dockerfile image building
Reworked Dockerfile to multistage image build
This PR must be merged ONLY after corresponding changes in model of Che:
eclipse-che/che#12489
Issue #71