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

[Misc] Moving deprecated Job APIs to legacy #482

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

surli
Copy link
Member

@surli surli commented Jul 25, 2023

No description provided.

@@ -19,7 +19,7 @@
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
-->

<org.xwiki.job.internal.DefaultJobStatus>
<org.xwiki.job.DefaultJobStatus>
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to modify the serialization in the XML files of the test, since the class is not present anymore in this module. I don't think it would have an impact for real world status serialization if the old class is available through the legacy module.

boolean ignore = false;
if (event instanceof JobStartedEvent) {
Job job = (Job) source;
if (job.getStatus() instanceof org.xwiki.job.AbstractJobStatus
Copy link
Member Author

Choose a reason for hiding this comment

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

I might have used an aspect for injecting this condition check, but it felt easier to do it like that.

@@ -46,7 +46,7 @@
import org.xwiki.job.event.JobStartedEvent;
import org.xwiki.job.event.status.JobStatus;
import org.xwiki.job.event.status.QuestionAnsweredEvent;
import org.xwiki.job.internal.AbstractJobStatus;
import org.xwiki.job.AbstractJobStatus;
Copy link
Member Author

Choose a reason for hiding this comment

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

This impacts a condition in #onJobStarted which is called from #onEvent hence the legacy class I added to support the condition with the old class.

Copy link
Member

@tmortagne tmortagne Jul 28, 2023

Choose a reason for hiding this comment

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

Those should have moved to org.xwiki.job.AbstractJobStatus back when it was introduced. There is no need to introduce any legacy for this change, as org.xwiki.job.internal.AbstractJobStatus is a org.xwiki.job.AbstractJobStatus too.

@surli surli requested a review from tmortagne July 25, 2023 08:24
@Named(ExtensionJobHistoryRecorder.NAME)
@Singleton
@Deprecated
public class LegacyExtensionJobHistoryRecorder extends ExtensionJobHistoryRecorder
Copy link
Member

Choose a reason for hiding this comment

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

Seems to me this class is actually useless.

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.

2 participants