Module talk:Commons link

Code review?

@RexxS: Remember the discussion we had on Mike Peel's talk page? I've implement Lua code to perform cross-checking against multiple Wikidata properties to look for Commons galleries or categories (or both). That is this module, which is currently in use at {{commons and category}} and {{commons and category-inline}}. I'd like to use it for {{commons-inline}}, and perhaps {{commons category}}. Before I do anything, would you be willing to look at the code and give me feedback? As usual, the testcases are here. Thanks for any help or suggestions you can provide! — hike395 (talk) 15:48, 15 March 2020 (UTC)

@Hike395: I've just checked the results at Module talk:Commons link/testcases and they look reasonably comprehensive, and absolutely accurate. I've also now gone through the code at Module:Commons link and it looks good – a lot of the code is similar to functions that I've used in the past, so I'm confident that your algorithms are correct. The only problems I foresee are when editors manage to misuse the code, and you need to decide whether it is better to let a function fail with a sensible error message or catch the error and let the function continue with a null result. For example:

• {{#invoke:Commons link |getGallery |qid=Q0 |search=}} → (← bad entity-id)
• {{#invoke:Commons link |getGallery |qid=Q68979196 |search=}} → [[Commons:Special:Search/|]] (← entity-id exists, but has no links)

You'll usually have to work out in your own mind whether it's possible to abuse the local functions, such as at line 15 qid = mw.wikibase.getEntityIdForCurrentPage() – is it possible for the code to be running on a page that has no linked EntityID? If so, what happens when qid is returned as nil on line 16? Similarly for line 20, if mw.wikibase.sitelink(qid) somehow manages to return nil (no sitelink). Incidentally, I'd recommend using the latest calls: mw.wikibase.getSitelink is current, while mw.wikibase.sitelink is a legacy alias and shouldn't be used in new code (per mw:Extension:Wikibase Client/Lua #Legacy aliases).

Anyway, I'd have no qualms about introducing the code into production, as long as you're ready to catch any feedback from editors who spot errors. To paraphrase von Moltke, "No code survives contact with the user." Cheers --RexxS (talk) 16:51, 15 March 2020 (UTC)

@RexxS: Thanks for the code review! Most of the code has nil checks (because I copied a lot of code fragments from your code in Module:WikidataIB). I've cleaned up the remaining problems that I could find, and added tests to cover the problems you found. Next, I'll propose use at Template talk:Commons-inline. Thanks again! — hike395 (talk) 00:40, 16 March 2020 (UTC)