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

morebits: batchOperation: minor improvements #782

Merged
merged 3 commits into from Jan 12, 2020

Conversation

siddharthvp
Copy link
Member

@siddharthvp siddharthvp commented Dec 9, 2019

1st commit: doc fixes.
2nd commit: execute postFinish() even when the pageList is empty. This situation could occur when the pageList used is variable.
3rd commit: allow individual status lines even if the worker function is not using Morebits.wiki.page or Morebits.wiki.api.

These will not have any effect on Twinkle.

…empty.

When the pageList is empty, a quick abort results. Ideally, the postFinish()
function should be executed even in this case.
@siddharthvp siddharthvp changed the title morebits: batchOperation: minor improvements to morebits: batchOperation: minor improvements Dec 9, 2019
ctx.running = false;
if (ctx.postFinish) {
ctx.postFinish();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is better than modifying fnDoneOne or something, if only for simplicity's sake, but even workerFailure calls fnDoneOne. It's a weird construct there overall.

Copy link
Collaborator

@Amorymeltzer Amorymeltzer left a comment

Choose a reason for hiding this comment

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

All looks pretty clear to me! One minor suggested change, but otherwise looks great. I don't love arg but can't come up with anything better that's still concise.

morebits.js Show resolved Hide resolved
If using mw.Api() (or anything other than Morebits.wiki.page/api) for processing pages in the worker, till now it was not possible to have invidual status lines for each page. This commit rectifies this deficiency by allowing a string input to workerSuccess() that would be treated as the page name, triggering individual status line generation.

I've changed the condition `if (apiobj && apiobj.getStatusElement)` to `if (apiobj instanceof Morebits.wiki.page || apiobj instanceof Morebits.wiki.api)`. These are functionally same, but the latter is more explicit and clearer.
@Amorymeltzer Amorymeltzer dismissed their stale review January 12, 2020 17:51

Change made, seems fine

@Amorymeltzer Amorymeltzer added this to the January 2020 update milestone Jan 12, 2020
@Amorymeltzer Amorymeltzer merged commit 241588c into wikimedia-gadgets:master Jan 12, 2020
@siddharthvp siddharthvp deleted the batchopfixes branch October 22, 2020 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants