486081 - nsTime should be removed

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 1

[Yu-Hsun Lin]

Attachment: patch that removes nsTime.h and it's related lines

patch

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

Great, thanks!

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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...

Comment 5

[Steffen Wilberg]

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

Comment 6

[Serge Gautherie (:sgautherie)]

(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".)

Comment 7

[Benjamin Smedberg]

Mailnews should import the nsTime code if it wishes to continue using it.

Comment 8

[timeless]

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

[Steffen Wilberg]

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

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Attachment: An rebased version

Thunderbird doesn't seem to be using nsTime anymore so I plan on landing this.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Attachment: A rebased version

Comment 12

[Blake Winton (:bwinton) (:☕️)]

Pushed to c-c try at http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=d2995582b09c
Also building on my local Windows box.

Comment 13

[Blake Winton (:bwinton) (:☕️)]

Or, you know, http://build.mozillamessaging.com/tinderboxpushlog/?tree=ThunderbirdTry&rev=c5a83a2482b0 with the added patch that will actually work.

Comment 14

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/3c155b728fe8