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

LPS-68989 WAB bundles needs to search resources in /META-INF/resources too #44499

Closed
wants to merge 2 commits into from

Conversation

rotty3000
Copy link

No description provided.

@liferay-continuous-integration
Copy link
Collaborator

All tests PASSED.

Build Time: 2 hours 18 minutes 14 seconds

Base Branch:

Branch Name: master
Branch GIT ID: 7244a67c8a75541358145cc81147bffafb48eab2

Job Summary:

For more details click here.
For upstream results, click here.

@brianchandotcom
Copy link
Owner

Merged. Thx.

@brianchandotcom
Copy link
Owner

@juangon isn't

            catch (IOException ioe) {
            }

a bad idea?

@hhuijser is going through and removing all swallowed exceptions.

@juangon can you send me a follow up and at least add a debug? or something else.

Thx.

@hhuijser
Copy link

hhuijser commented Nov 4, 2016

@brianchandotcom @juangon right now, SF only complains about swallowed exceptions that extend PortalException or SystemException.

@Preston-Crary @shuyangzhou do we want to check for any type of exception, at some point?

@shuyangzhou
Copy link

Yes @hhuijser , eventually, when we have the bandwidth for it, not going to happen anytime soon.

@hhuijser
Copy link

hhuijser commented Nov 4, 2016

Just FYI, we have a little over 1100 cases in master that we'd need to fix...

@shuyangzhou
Copy link

Ok, that's a lot, but I was preparing for worse. That's ok, we will get to them eventually. Thanks @hhuijser !

@juangon
Copy link

juangon commented Nov 4, 2016

@brianchandotcom @hhuijser @shuyangzhou yep, as I didn't want to change the API, and the following version increase etc, I looked through Liferay and saw other cases like here: https://github.com/liferay/liferay-portal/blob/master/modules/apps/foundation/portal/portal-spring-extender/src/main/java/com/liferay/portal/spring/extender/internal/classloader/BundleResolverClassLoader.java#L80-L88 and here: https://github.com/liferay/liferay-portal/blob/master/modules/apps/foundation/portal-template/portal-template-freemarker/src/main/java/com/liferay/portal/template/freemarker/internal/FreeMarkerBundleClassloader.java#L65-L75. I am not a fan of swallowing exceptions, but I think in this specific case, even throwing it, it wouldn't be possible to take any action for recovering. Maybe an error log would be fine. I let that decision to you.

@Preston-Crary
Copy link

@juangon Please review this documentation https://issues.liferay.com/browse/LPS-36174?focusedCommentId=882129#comment-882129 if you're not sure how to handle an exception. I think the best thing to do in this case is log a warning or maybe throw an unchecked exception depending on how serious you think this failure would be to the rest of the system.

@juangon
Copy link

juangon commented Nov 4, 2016

Hi @Preston-Crary, I will probably add a debug, will think about it next week.
Thanks very much for that guidelines!

@juangon juangon mentioned this pull request Nov 7, 2016
@juangon
Copy link

juangon commented Nov 7, 2016

Sent here: #44552

@juangon
Copy link

juangon commented Nov 22, 2016

In final review here: #44967

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.

7 participants