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

KOGITO-1661 : BPMN Examples have 'null' in process type property #3258

Merged
merged 1 commit into from
Jul 3, 2020

Conversation

inodeman
Copy link
Contributor

@inodeman inodeman commented Apr 3, 2020

Hi @romartin can you review this PR?

@inodeman
Copy link
Contributor Author

inodeman commented Apr 3, 2020

Jenkins execute full downstream build

@sonarcloud
Copy link

sonarcloud bot commented Apr 4, 2020

Kudos, SonarCloud Quality Gate passed!

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@romartin
Copy link
Contributor

romartin commented Apr 6, 2020

Hey @tomasdavidorg

Could you please quickly review this fix for https://issues.redhat.com/browse/KOGITO-1661 (you reported it)? this way we can include into next release asap.

Thanks!

@@ -242,7 +242,7 @@
* @ordered
*/
@GwtTransient
protected ProcessType processType = PROCESS_TYPE_EDEFAULT;
Copy link
Contributor

Choose a reason for hiding this comment

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

@inodeman so the fix is just about initializing the processType with the default public value, right? Sounds ok for me, but could then we also remove the field PROCESS_TYPE_EDEFAULT, is being used somewhere else?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is used on another places, shouldn't be the PROCESS_TYPE_EDEFAULT set to ProcessType.PUBLIC?

Copy link
Contributor

Choose a reason for hiding this comment

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

good point @tomasdavidorg - @inodeman ?? ^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @romartin @tomasdavidorg I tried setting the default to public, but the side effect is that if a process is set to public, then it is not saved. Then to make it work I have to change several files. Let me know

Copy link
Contributor

@romartin romartin Apr 8, 2020

Choose a reason for hiding this comment

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

@inodeman TBH I think it results confusing having the PROCESS_TYPE_EDEFAULT field, but not using it for the initialization of processType, although is actually working this way. Ping me and we can do a quick look at this together, np! 👍

@inodeman
Copy link
Contributor Author

Jenkins execute full downstream build

@inodeman
Copy link
Contributor Author

Jenkins retest please

@inodeman
Copy link
Contributor Author

Hi @romartin @LuboTerifaj can you review this PR?

@sonarcloud
Copy link

sonarcloud bot commented May 30, 2020

SonarCloud Quality Gate failed.

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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (1.8.0_202) you have used to run this analysis is deprecated and we will stop accepting it from October 2020. Please update to at least Java 11.
Read more here

@inodeman
Copy link
Contributor Author

inodeman commented Jun 2, 2020

Jenkins execute full downstream build

@domhanak
Copy link
Contributor

domhanak commented Jul 1, 2020

@tomasdavidorg @romartin any updates?

@romartin romartin requested review from domhanak and removed request for tomasdavidorg July 2, 2020 11:40
@romartin
Copy link
Contributor

romartin commented Jul 2, 2020

Jenkins execute full downstream build

@romartin
Copy link
Contributor

romartin commented Jul 2, 2020

please @inodeman , keep an eye on the builds and once ready, let reviews know so we can complete the review. Thanks!

@romartin romartin merged commit a9d9aee into kiegroup:master Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants