Skin.js has become large and appears to be operating more like an Application then a skin. We may want to revisit this file, pull things out of it, move it out of mobile.startup and maybe even move some of it to skins.minerva.script.
Let's chat about the code as a group and see how we can improve this part of the codebase.
Outcome: A plan for refactoring (if any)
Inform and update T173454
Duration: 2hrs
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Jdlrobson | T166905 [EPIC] Talk about and improve our frontend code architecture | |||
| Resolved | Jdlrobson | T138764 Spike: Do a code review session on Skin.js |
This is still relevant. I'm hoping we can get round to this soon... I fear our PHP code needs some love first.
Outputs:
There was an interesting conversation started by Piotr about our use of comments in our projects. We came to the conclusion that inline comments are usually only useful if they describe "why" things are the way they are.
e.g. https://github.com/wikimedia/mediawiki-skins-MinervaNeue/blob/master/resources/skins.minerva.tablet.scripts/toc.js#L27
we felt the comment should describe why a table of contents would be there - not explain the line that followed.
I think this was a useful session. We only spent an hour here, but I think the outputs should be to focus on the 3 tasks mentioned above.