nsTime should be removed

RESOLVED FIXED in mozilla8

Status

()

Core
XPCOM
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: roc, Assigned: Yu-Hsun Lin)

Tracking

Trunk
mozilla8
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

http://mxr.mozilla.org/mozilla-central/search?string=nstime[^A-Za-z]&regexp=on&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

It looks like the only actual uses of this is some code that's #if defined(DEBUG_waterson) || defined(DEBUG_hyatt). That should just be deleted.
Whiteboard: [good first bug]
Assignee: nobody → mardeg
Status: NEW → ASSIGNED
(Assignee)

Comment 1

8 years ago
Created attachment 392638 [details] [diff] [review]
patch that removes nsTime.h and it's related lines

patch
Comment on attachment 392638 [details] [diff] [review]
patch that removes nsTime.h and it's related lines

Great, thanks!
Attachment #392638 - Flags: review+
In future you should request review for your patches, from a particular person. If you ask around someone will tell you who, if you don't know.
Keywords: checkin-needed
Whiteboard: [good first bug] → [needs landing]
http://tinderbox.mozilla.org/showlog.cgi?log=Thunderbird/1249440407.1249442340.13431.gz

Thunderbird still uses nsTime, apparently, so I backed this out :-(.

I wonder who else might still be using it...
Keywords: checkin-needed
Whiteboard: [needs landing]
Assignee: mardeg → oceanl

Comment 5

8 years ago
Use http://mxr.mozilla.org/comm-central/, which includes both mozilla-central and comm-central:
http://mxr.mozilla.org/comm-central/search?string=nstime[^A-Za-z]&regexp=on&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
(In reply to comment #5)
> Use http://mxr.mozilla.org/comm-central/, which includes both mozilla-central
> and comm-central:

Fwiw, currently it's c-c + m-1.9.1 (+ others).

***

http://mxr.mozilla.org/comm-central/search?string=nsTime&case=on&find=%2Fmailnews%2F
Can you file a MailNews Core bug about this? (assuming it's to be "fixed".)
Mailnews should import the nsTime code if it wishes to continue using it.

Updated

8 years ago
Depends on: 508558

Comment 8

8 years ago
to make searching bugzilla easier, roc's backout is:
http://hg.mozilla.org/mozilla-central/rev/1f823d3b7fbf
"Backoug but 586081, mailnews still uses nsTime"

Comment 9

6 years ago
Mailnews is still working on getting rid of nsTime in bug 508558. But in the meantime, bug 435296 started using it in /modules/libpr0n/src/RasterImage.cpp:
http://mxr.mozilla.org/mozilla-central/search?string=nstime&find=RasterImage.cpp&tree=mozilla-central
Created attachment 546585 [details] [diff] [review]
An rebased version

Thunderbird doesn't seem to be using nsTime anymore so I plan on landing this.
Attachment #392638 - Attachment is obsolete: true
Created attachment 546589 [details] [diff] [review]
A rebased version
Attachment #546585 - Attachment is obsolete: true
Pushed to c-c try at http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=d2995582b09c
Also building on my local Windows box.
Or, you know, http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=c5a83a2482b0 with the added patch that will actually work.
http://hg.mozilla.org/mozilla-central/rev/3c155b728fe8
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
You need to log in before you can comment on or make changes to this bug.