Niedzielski has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/369966 )
Change subject: Comments from code review session
......................................................................
Comments from code review session
Change-Id: I9290e10103e3e0011f95ddb5eab3355e8f9eee17
---
M resources/mobile.talk.overlays/TalkOverlay.js
M resources/mobile.talk.overlays/TalkSectionAddOverlay.js
2 files changed, 15 insertions(+), 2 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/66/369966/1
diff --git a/resources/mobile.talk.overlays/TalkOverlay.js
b/resources/mobile.talk.overlays/TalkOverlay.js
index f0c4cdb..2c63ec7 100644
--- a/resources/mobile.talk.overlays/TalkOverlay.js
+++ b/resources/mobile.talk.overlays/TalkOverlay.js
@@ -1,9 +1,21 @@
+// bundler doesn't require closure to get nice variable naming. todo: maybe we
can get rid of closures?
+// todo: email on how people have their ides configured for completions.
+// todo: get consensus on naming the MediaWiki object. some places we call it
M for magic (really a
+// service locator-- just a global dictionary of objects that everybody
references), other places we
+// call it MW for MediaWiki.
+// todo: (sam, jon, stephen) user should be passed as an argument.
( function ( M, $ ) {
+ // todo: lint. Require spacing between functions at least. And...?
+ // do these just become imports in marvin (with bundler)?
var TalkOverlayBase = M.require( 'mobile.talk.overlays/TalkOverlayBase'
),
Page = M.require( 'mobile.startup/Page' ),
Anchor = M.require( 'mobile.startup/Anchor' ),
user = M.require( 'mobile.startup/user' );
- /**
+
+ // https://phabricator.wikimedia.org/T156186
+ // lots of discussion on M/MW, particularly M.on / M.emit. stephen: i
need more discussion on mw.msg() and i like event busses
+
+ /**
* Overlay for talk page
* @extends Overlay
* @class TalkOverlay
@@ -153,6 +165,7 @@
}
} );
+ // asynchronously load sources..?
M.define( 'mobile.talk.overlays/TalkOverlay', TalkOverlay ); //
resource-modules-disable-line
}( mw.mobileFrontend, jQuery ) );
diff --git a/resources/mobile.talk.overlays/TalkSectionAddOverlay.js
b/resources/mobile.talk.overlays/TalkSectionAddOverlay.js
index 6ac3b74..def0229 100644
--- a/resources/mobile.talk.overlays/TalkSectionAddOverlay.js
+++ b/resources/mobile.talk.overlays/TalkSectionAddOverlay.js
@@ -106,7 +106,7 @@
M.emit( 'talk-discussion-added'
);
window.history.back();
} else {
- M.emit( 'talk-added-wo-overlay'
);
+ M.emit( 'talk-added-wo-overlay'
); // todo: no one subscribes. can we remove this? (sam, jon)
}
}
} ).fail( function ( error ) {
--
To view, visit https://gerrit.wikimedia.org/r/369966
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9290e10103e3e0011f95ddb5eab3355e8f9eee17
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Niedzielski <[email protected]>
Gerrit-Reviewer: Sniedzielski <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
