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.

Sign up for GitHub

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

Jump to bottom

Handle more fields in plugin meta YAML #12489

Merged
merged 1 commit into from
Feb 12, 2019
Merged

Handle more fields in plugin meta YAML #12489

merged 1 commit into from
Feb 12, 2019

Conversation

mkuznyetsov
Copy link
Contributor

@mkuznyetsov mkuznyetsov commented Jan 22, 2019 •
edited
Loading

What does this PR do?

Add more possible fields for plugin yaml (not validated anyhow atm), for use in plugin viewer:
category
publisher
repository
tags
mediaImage
mediaVideo
firstPublicationDate
latestUpdateDate
preview

What issues does this PR fix or reference?

eclipse-che/che-plugin-registry#71

Release Notes

Docs PR

@che-bot che-bot added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Jan 22, 2019
skabashnyuk
skabashnyuk reviewed Jan 22, 2019
View reviewed changes
Copy link
Contributor

@skabashnyuk skabashnyuk left a comment

Choose a reason for hiding this comment

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

LGTM @garagatyi wdyt?

@mkuznyetsov mkuznyetsov mentioned this pull request Jan 23, 2019
Merged
@mkuznyetsov
Copy link
Contributor Author

ci-test

@che-bot
Copy link
Contributor

che-bot commented Jan 23, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:12489
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

garagatyi
garagatyi reviewed Jan 24, 2019
View reviewed changes
...workspace/src/main/java/org/eclipse/che/api/workspace/server/wsplugins/model/PluginMeta.java Outdated
@@ -153,6 +154,9 @@ public PluginMeta tags(List<String> tags) {
}

public List<String> getTags() {
if (tags == null) {
return new ArrayList<>();
Copy link

Choose a reason for hiding this comment

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

It should be assigned

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, also added initialization in the field itself

@mkuznyetsov
Copy link
Contributor Author

ci-test

@che-bot
Copy link
Contributor

che-bot commented Jan 24, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:12489
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@mkuznyetsov
Copy link
Contributor Author

@garagatyi fyi, as @sleshchenko said, I decided to remove initialization in all fields in the class, since they are lazily initialized in getters anyway, and so would be consistent for all fields

@skabashnyuk
Copy link
Contributor

@ibuziuk FYI

@mkuznyetsov
Copy link
Contributor Author

ci-test

@mkuznyetsov
Copy link
Contributor Author

@garagatyi @sleshchenko I have also a proposition to add @JsonIgnoreProperties(ignoreUnknown=true)
annotation, so that we would be able to allow to have fields in plugin yamls, that are not defined in model class. This may prevent Che from failing to start workspaces, which ends up using plugin yaml that may have such fields, in case there will be new ones added in future

@ibuziuk ibuziuk mentioned this pull request Jan 28, 2019
Closed
@che-bot
Copy link
Contributor

che-bot commented Jan 28, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:12489
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@garagatyi
Copy link

Please, sign your commits.
I'm OK with the suggested solution and OK to do it in this PR or another one.

@sleshchenko
Copy link
Member

I have also a proposition to add @JsonIgnoreProperties(ignoreUnknown=true)

I'm OK with your proposal.

@mshaposhnik mshaposhnik mentioned this pull request Jan 29, 2019
Merged
@mkuznyetsov
Copy link
Contributor Author

ci-test

@che-bot
Copy link
Contributor

che-bot commented Feb 4, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:12489
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@artaleks9
Copy link
Contributor

Selenium tests execution on Eclipse Che Multiuser on OCP (https://ci.codenvycorp.com/job/che-pullrequests-test-ocp/1506/) doesn't show any regression against this Pull Request.

@mkuznyetsov
Add fields for plugin viewer in plugin meta model
280e11d 
Signed-off-by: Mykhailo Kuznietsov <mkuznets@redhat.com>
@mkuznyetsov mkuznyetsov merged commit fa07afc into master Feb 12, 2019
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 12, 2019
@che-bot che-bot added this to the 6.19.0 milestone Feb 12, 2019
@skabashnyuk skabashnyuk deleted the plugin-registry-71 branch March 18, 2019 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skabashnyuk skabashnyuk skabashnyuk approved these changes

@garagatyi garagatyi garagatyi approved these changes

@sleshchenko sleshchenko sleshchenko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
6.19.0
Development

Successfully merging this pull request may close these issues.
6 participants
@mkuznyetsov @che-bot @skabashnyuk @garagatyi @sleshchenko @artaleks9