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 58449 Refactor scheduler related classes #1772

Closed
wants to merge 82 commits into from
Closed

Lps 58449 Refactor scheduler related classes #1772

wants to merge 82 commits into from

Conversation

tinatian
Copy link

@tinatian tinatian commented Sep 9, 2015

Hi Shuyang,

With these changes, a NumberFormatException would come up while starting portal, it is caused by an old bug(see https://issues.liferay.com/browse/LPS-58479), will send pull request to Micheal Han who made those changes before.

Tina.

brianchandotcom and others added 30 commits September 8, 2015 14:04
…ilable if the NodeJS version matches the OS arch
@liferay-continuous-integration
Copy link
Collaborator

Some tests FAILED!

Build Time: 1 hour 30 minutes 14 seconds

Job Results:

17 Jobs Passed.
1 Job Failed.

See the following links for more information.

@tinatian
Copy link
Author

ci:retest

@shuyangzhou
Copy link
Owner

It is ok, the build failure is not caused by you. I just did not have time to review it yet, will try to get on to it tomorrow.

@liferay-continuous-integration
Copy link
Collaborator

Some tests FAILED!

Build Time: 55 minutes 26 seconds

Job Results:

16 Jobs Passed.
2 Jobs Failed.

See the following links for more information.

@shuyangzhou
Copy link
Owner

I don't really like the TriggerContent design, that is technically the same as what we currently have with TriggerType, after all in the end you still have to instance of to test the actual TriggerContent type to behave differently.

I think we should create our own TriggerBuilder interface/impl, bridge it to Quartz's TriggerBuilder at impl level. At least this way we don't have to do instance of any more.

If you think otherwise, please resend, then we can talk about it.

@tinatian
Copy link
Author

Em....I will think about your suggestion, will resend pull request about this.

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.