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

[JBPM-9410] No Model definition in KIE Server Swagger API #2263

Merged
merged 1 commit into from
Oct 9, 2020
Merged

[JBPM-9410] No Model definition in KIE Server Swagger API #2263

merged 1 commit into from
Oct 9, 2020

Conversation

fjtirado
Copy link
Contributor

@fjtirado fjtirado commented Oct 5, 2020 •
edited
Loading

Moving response to ApiResponse

JIRA:

JBPM-9410

@fjtirado fjtirado changed the title [RPHAM-3219] No Model definition in KIE Server Swagger API [RHPAM-3219] No Model definition in KIE Server Swagger API Oct 5, 2020
elguardian
elguardian requested changes Oct 6, 2020
View reviewed changes
Copy link
Member

@elguardian elguardian left a comment •
edited
Loading

Choose a reason for hiding this comment

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

please use jbpm jira, not rhpam

@fjtirado
Copy link
Contributor Author

fjtirado commented Oct 6, 2020

please use jbpm jira, not rhpam

Can you specify a clear criteria for using RHPAM or JBPM (since for other RHPAM we have used directly the RHPAM to commit changes? Once a PR is created, switching from RHPAM to JBPM is a burden.

@fjtirado fjtirado changed the title [RHPAM-3219] No Model definition in KIE Server Swagger API [JBPM-9410] No Model definition in KIE Server Swagger API Oct 6, 2020
elguardian
elguardian approved these changes Oct 7, 2020
View reviewed changes
Copy link
Member

@elguardian elguardian left a comment

Choose a reason for hiding this comment

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

Ok. LGTM please try to avoid reformatting imports when they are not needed.

afalhambra
afalhambra suggested changes Oct 7, 2020
View reviewed changes
Copy link
Contributor

@afalhambra afalhambra left a comment

Choose a reason for hiding this comment

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

Very nice work. The only remarkable change I'm missing is the kie-server-rest-case-mgmt module which has not been modified to include this change.

I also wanted to know what @MarianMacik thinks regarding revapi config module, though I think that should be ok.

Thanks @fjtirado!

kie-server-parent/kie-server-remote/kie-server-rest/kie-server-rest-jbpm/pom.xml
Comment on lines +177 to +179
<configuration>
<failSeverity>breaking</failSeverity>
</configuration>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this makes sense, but I need to ask @MarianMacik about it as he's been working with this more closely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, let me know if there is other way. Adding all ignores does not makes sense

...t/kie-server-rest-jbpm/src/main/java/org/kie/server/remote/rest/jbpm/DefinitionResource.java Outdated Show resolved Hide resolved
.../kie-server-rest-jbpm/src/main/java/org/kie/server/remote/rest/jbpm/RuntimeDataResource.java Show resolved Hide resolved
.../kie-server-rest-jbpm/src/main/java/org/kie/server/remote/rest/jbpm/RuntimeDataResource.java Show resolved Hide resolved
@fjtirado
Copy link
Contributor Author

fjtirado commented Oct 7, 2020

Very nice work. The only remarkable change I'm missing is the kie-server-rest-case-mgmt module which has not been modified to include this change.

Sure, Ill change those ones too

afalhambra
afalhambra approved these changes Oct 7, 2020
View reviewed changes
Copy link
Contributor

@afalhambra afalhambra left a comment

Choose a reason for hiding this comment

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

Great work. Only successful message hasn't been updated for case-mgmt. Sorry about it.
Other than that it's perfect.

...rver-rest-case-mgmt/src/main/java/org/kie/server/remote/rest/casemgmt/CaseAdminResource.java Outdated Show resolved Hide resolved
afalhambra
afalhambra reviewed Oct 7, 2020
View reviewed changes
Copy link
Contributor

@afalhambra afalhambra left a comment

Choose a reason for hiding this comment

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

Great work! thanks a lot @fjtirado

@sonarcloud
Copy link

sonarcloud bot commented Oct 7, 2020

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

100.0% 100.0% Coverage
7.6% 7.6% Duplication

gmunozfe
gmunozfe approved these changes Oct 9, 2020
View reviewed changes
Copy link
Member

@gmunozfe gmunozfe left a comment

Choose a reason for hiding this comment

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

looks good, fantastic work @fjtirado !

@fjtirado
Copy link
Contributor Author

fjtirado commented Oct 9, 2020

@kiegroup/gatekeepers please merge this

@mareknovotny mareknovotny merged commit 3ca9ff8 into kiegroup:master Oct 9, 2020
@MarianMacik MarianMacik mentioned this pull request Oct 22, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmunozfe gmunozfe gmunozfe approved these changes

@elguardian elguardian elguardian approved these changes

@afalhambra afalhambra afalhambra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@fjtirado @gmunozfe @elguardian @afalhambra @mareknovotny