| CARVIEW |
Navigation Menu
-
-
Notifications
You must be signed in to change notification settings - Fork 8
Add support to Liferay Portal version 7.0 #7
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
Conversation
| @@ -5,7 +5,7 @@ | |||
|
|
|||
| <groupId>net.bull.javamelody</groupId> | |||
| <artifactId>liferay-javamelody-hook</artifactId> | |||
| <version>1.63.0</version> | |||
| <version>1.64.0</version> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not put 1.64.0 yet.
| <source>1.6</source> | ||
| <target>1.6</target> | ||
| <source>1.8</source> | ||
| <target>1.8</target> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing java version is not necessary.
| <groupId>org.quartz-scheduler</groupId> | ||
| <artifactId>quartz</artifactId> | ||
| <version>2.2.3</version> | ||
| </dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose that adding the quartz dependency was done in order to disable quartz monitoring.
But it would be simpler and better to change the following method in the JobInformations class:
private static boolean isQuartzAvailable() {
try {
Class.forName("org.quartz.Job");
Class.forName("org.quartz.impl.SchedulerRepository");
return true;
} catch (final ClassNotFoundException e) {
return false;
}
}| import com.liferay.portal.util.PortalUtil; | ||
| import com.liferay.portal.kernel.model.RoleConstants; | ||
| import com.liferay.portal.kernel.service.UserLocalServiceUtil; | ||
| import com.liferay.portal.kernel.util.PortalUtil; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hook needs to be backward compatible with Liferay 6.1+.
So the previous dependency portal-service should be added back and the code should use one or the other classes are available at runtime.
If think we can do something like:
private boolean liferay7;
try {
if (!liferay7) {
// do for liferay 6
}
} catch (final ClassNotFoundException e) {
liferay7 = true;
}
if (liferay7) {
// do for liferay 7
}| @@ -8,7 +8,7 @@ page-url=https://github.com/javamelody/javamelody/wiki | |||
| author=Emeric Vernat | |||
| licenses=ASL | |||
| # compatibility "6.1+" is currently like "6.1.1+,6.1.20+,6.2.0+" (6.1 CE GA2+,6.1 EE GA2+, and 6.2 CE GA1+) | |||
| liferay-versions=6.1+ | |||
| liferay-versions=7.0.0+ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6.1+,7.0.0+ is better to be backward compatible
|
Thanks for notifying us about Liferay changes and also for submitting pull request. That's much appreciated. But I think that it is a bad idea from Liferay to cut libraries apis in pieces (some api parts are no longer available), that's not respectful of api designers. And I think it is a bad idea from Liferay to rename Liferay packages without any backward compatibility, that's not respectful of developers. As said in reviews, the PR needs some changes to be merged. |
|
I will merge and restore Liferay 6.1+ compatibility after that. |
|
Released as 1.63.1 of liferay-javamelody: |
Hi
this pull only will works with this other javamelody/javamelody#611.
Thanks!