Closed
Bug 1025548
Opened 10 years ago
Closed 10 years ago
Preliminary perf/code org tweaks for Bug 257037
Categories
(MailNews Core :: Feed Reader, defect)
MailNews Core
Feed Reader
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 33.0
People
(Reporter: alta88, Assigned: alta88)
References
Details
Attachments
(4 files, 4 obsolete files)
(deleted),
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mkmelin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
alta88
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
alta88
:
review+
|
Details | Diff | Splinter Review |
The Bug 257037 patch contained a number of tangential changes which are better to split out for ease of review etc.
Part1: implement isBiff in the interface and move, word for word, downloadFeed() and subscribeToFeed() to FeedUtils.jsm. updateSubscriptionsDS() has already been moved.
Part2: reorg/genericise some functions.
Part3: some perf/snappy tweaks, and active delete of dead objects, for Bug 950561 type cases.
Assignee: nobody → alta88
Attachment #8440327 -
Flags: review?(mkmelin+mozilla)
Attachment #8440328 -
Flags: review?(mkmelin+mozilla)
Attachment #8440329 -
Flags: review?(mkmelin+mozilla)
Attachment #8440329 -
Attachment is obsolete: true
Attachment #8440329 -
Flags: review?(mkmelin+mozilla)
Attachment #8442095 -
Flags: review?(mkmelin+mozilla)
Part 4 - move FeedMessageHandler to newsblogOverlay.js where i should have put it in the first place; UTF8 tweaks.
Attachment #8448045 -
Flags: review?(mkmelin+mozilla)
Updated•10 years ago
|
Attachment #8440327 -
Flags: review?(mkmelin+mozilla) → review+
Comment 6•10 years ago
|
||
Comment on attachment 8440328 [details] [diff] [review]
feedUpdatesPart2.patch
Review of attachment 8440328 [details] [diff] [review]:
-----------------------------------------------------------------
::: mailnews/extensions/newsblog/content/FeedUtils.jsm
@@ +480,5 @@
> }
>
> return feedUrlArray.length ? feedUrlArray : null;
> },
> +
whitespace
::: mailnews/extensions/newsblog/content/feed-subscriptions.js
@@ +54,4 @@
> {
> + this.mMainWin.FeedFolderNotificationService = MailServices.mfn;
> + this.mMainWin.FeedFolderNotificationService.
> + addListener(this.FolderListener, this.FOLDER_ACTIONS);
please when breaking lines, put . on the new line
Attachment #8440328 -
Flags: review?(mkmelin+mozilla) → review+
Updated•10 years ago
|
Attachment #8442095 -
Flags: review?(mkmelin+mozilla) → review+
Comment 7•10 years ago
|
||
Comment on attachment 8448045 [details] [diff] [review]
feedUpdatesPart4.patch
Review of attachment 8448045 [details] [diff] [review]:
-----------------------------------------------------------------
While touching this you could fix the double spacing in documentations, and the space before function (e.g. function (aFoo....) -> function(aFoo
Attachment #8448045 -
Flags: review?(mkmelin+mozilla) → review+
Attachment #8440328 -
Attachment is obsolete: true
Attachment #8451357 -
Flags: review+
Attachment #8448045 -
Attachment is obsolete: true
Attachment #8451358 -
Flags: review+
Assignee | ||
Comment 10•10 years ago
|
||
updated parts 2 and 4 for comments; moved gShowFeedSummary global into newsblogOverlay.js.
Keywords: checkin-needed
Comment 11•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/60013aa5437b
https://hg.mozilla.org/comm-central/rev/647a2915cf66
https://hg.mozilla.org/comm-central/rev/face6ed1726e
https://hg.mozilla.org/comm-central/rev/01b838f9a078
Status: NEW → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 33.0
Comment 12•10 years ago
|
||
Backed out for Mozmill failures.
https://hg.mozilla.org/comm-central/rev/b64ae233cdde
https://tbpl.mozilla.org/php/getParsedLog.php?id=43749551&tree=Thunderbird-Trunk
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Thunderbird 33.0 → ---
Assignee | ||
Comment 13•10 years ago
|
||
part4 was missing an include in SearchDialog.xul.
Attachment #8451358 -
Attachment is obsolete: true
Attachment #8455647 -
Flags: review+
Keywords: checkin-needed
Comment 14•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/16b1ba92e2ff
https://hg.mozilla.org/comm-central/rev/3040b96c3d34
https://hg.mozilla.org/comm-central/rev/ece22ed6d733
https://hg.mozilla.org/comm-central/rev/ab5280bd88bc
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 33.0
Comment 15•10 years ago
|
||
Backed out for the test-message-header.js perma-fail seen in the log below (not the known perma-fail tracked in Bug 1038647). Please verify that this is green on Try before requesting checkin again.
https://hg.mozilla.org/comm-central/rev/43c7f777a37f
https://tbpl.mozilla.org/php/getParsedLog.php?id=43834979&tree=Thunderbird-Trunk
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Thunderbird 33.0 → ---
Comment 16•10 years ago
|
||
This might actually be a new perma-fail from m-c. Still investigating.
Assignee | ||
Comment 17•10 years ago
|
||
Thanks for the notice. (This code doesn't touch anything near header display and couldn't be the culprit for that particular test failure).
I'll take the liberty of adding checkin-needed to get on the queue once mozmill is sorted; you can adjust as you see fit.
Keywords: checkin-needed
Comment 18•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/386ade37ed80
https://hg.mozilla.org/comm-central/rev/959819e017a4
https://hg.mozilla.org/comm-central/rev/92f45cc08377
https://hg.mozilla.org/comm-central/rev/72f40b5cea35
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 33.0
You need to log in
before you can comment on or make changes to this bug.
Description
•