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-780: [DMN Designer] Ensure -runtime WAR is created with 'production' parameters #3081

Merged
merged 3 commits into from
Jan 2, 2020

Conversation

manstis
Copy link
Member

@manstis manstis commented Dec 18, 2019

See https://issues.redhat.com/browse/KOGITO-780..

Basically I removed the draftCompile flag and a duplicate logLevel entry.

@manstis manstis requested review from jomarko, ederign and tiagobento and removed request for ederign December 18, 2019 13:29
@tiagobento
Copy link
Contributor

@manstis I think we should include <style>OBFUSCATED</style> and probably <optimizationLevel>9</optimizationLevel> as well.

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@tiagobento Updates made as proposed.

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@yesamer @romartin FYI these are the settings we're using for DMN... The -runtime WAR is ~64.6MB however WEB-INF is ~64.2MB (and excluded from the plugin) meaning the plugin itself is 0.4MB or ~400kb!

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@yesamer @romartin Ignore the sizes I gave above... I was comparing oranges to apples.

@tiagobento
Copy link
Contributor

@manstis on previous builds, DMN editor resources are ~26MB

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@tiagobento What are "DMN editor resources"? The WAR? The exploded WAR less WEB-INF?

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@tiagobento Do you have a tag/label that I can checkout to investigate? Nothing of any significance has been added explicitly to DMN to warrant a x3 size increase (-runtime WAR is now 66MB).

@tiagobento
Copy link
Contributor

@manstis That's the exploded WAR less WEB-INF :)

@tiagobento
Copy link
Contributor

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@tiagobento It's 36MB with this PR...

@tiagobento
Copy link
Contributor

@manstis I guess that's because there's gecko on the compilation targets? We only need chrome..

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@tiagobento Correct... I finally found the difference (9MB for another permutation).

I'll update this PR to just compile Chrome.

@manstis
Copy link
Member Author

manstis commented Dec 20, 2019

@tiagobento ~26MB now :-)

@tiagobento
Copy link
Contributor

@manstis Thx! Looks perfect now.

@manstis
Copy link
Member Author

manstis commented Jan 2, 2020

@jomarko New Year reminder to please review this PR :-)

@manstis
Copy link
Member Author

manstis commented Jan 2, 2020

Jenkins please retest this.

Copy link

@jomarko jomarko left a comment

Choose a reason for hiding this comment

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

Tried to build VS code extension, open and edit file. It works.

@manstis manstis merged commit abd5a9f into kiegroup:master Jan 2, 2020
<!-- There is no "ie11" permutation. IE11 uses the Firefox one (gecko1_8) -->
<set-property name="user.agent" value="gecko1_8,safari"/>
<!-- We only need Chrome -->
<set-property name="user.agent" value="safari"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for my own reasoning - is that really true guys @manstis @ederign @porcelli ? For kogito webapps can we only expect support for safari engines? Is because our kogito channels are using only this implemention?

@porcelli
Copy link
Member

good catch @romartin, I think we need both.

@manstis
Copy link
Member Author

manstis commented Feb 18, 2020

@romartin @porcelli,

@tiagobento said we only needed Chrome (when we were discussing WAR sized).

@porcelli
Copy link
Member

@tiagobento is this true for the online editors? I mean... by using safari alone would still work in all browsers?

I know that it's true for VSCode, GitHub Extensions and Desktop

@tiagobento
Copy link
Contributor

@porcelli Well, you're right. I don't remember if at some point we decided to only support Chrome/Safari but we can revisit that. Keep in mind that supporting Firefox will increase the size of VSCode extension and Desktop bundles.

@porcelli
Copy link
Member

for VSCode, GitHub Extension and Desktop all we need is chrome support.

however.. online editor is a different thing.

@manstis
Copy link
Member Author

manstis commented Feb 18, 2020

@porcelli Sooooooo, it's your call..

We can add an additional profile to compile a WAR containing more permutations for online however that's the tip of the iceberg... How it's then compiled as part of the community build and how you consume it in kogito-tooling for the different channels would remain undetermined.

Please be sure to keep @romartin, @yesamer and myself informed of your decisions.

@yesamer
Copy link
Member

yesamer commented Feb 18, 2020

@manstis I had the same idea, we can use different profiles to handle different versions of gwt.xml with different user-agent. Undoubtedly, understand how kogito-tooling can take the correct version based on the channel is a big question. Anyway, I'm available to discuss more, at the moment I have both enabled for scesim, but I planned to remove gecko engine soon. Thanks!

@romartin
Copy link
Contributor

ok so it looks still unclear... :)

I'll use safari for now in BPMN as well, but please let us know guys if that should change, in some way, at some point... :)

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants