603253 - Make featureConfigurator.js use iteratorUtils.toArray

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Rain [:rain1] (inactive)]

Now that we've fixed toArray, we might as well make use of it in featureConfigurator.js. Blake, if you would upload that patch you have I could review it.

+++ This bug was initially created as a clone of Bug #559909 +++

Comment 1

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

Attachment: A patch to use the new iterator functionality.

(I haven't tested it yet, but I thought I'ld start it through the review process while I was waiting for the tests to run.)

Thanks,
Blake.

Comment 2

[Rain [:rain1] (inactive)]

Comment on attachment 482597 [details] [diff] [review]
A patch to use the new iterator functionality.

As you pointed out on IRC, you don't actually call toArray. :P

Comment 3

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

Attachment: How about this one?

It even passes the tests, although it is a little ugly to have to write:
servers = toArray(Iterator(servers));
instead of just:
servers = toArray(servers);

Thanks,
Blake.

Comment 4

[Rain [:rain1] (inactive)]

Comment on attachment 482667 [details] [diff] [review]
How about this one?

Iterator is the wrong thing to use here: http://grab.by/grabs/0f399aaaba4dffbb5668fc5cdb3d4bb7.png

Comment 5

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

Attachment: And this should hopefully be the final version.

(In reply to comment #4)
> Iterator is the wrong thing to use here:
> http://grab.by/grabs/0f399aaaba4dffbb5668fc5cdb3d4bb7.png

Indeed.  fixIterator seems to work, though, as you mentioned on IRC.

(I printed out the server type in a dump statement, and it returned "none", and "pop3", so I believe it works.)

Thanks,
Blake.

Comment 6

[Rain [:rain1] (inactive)]

Comment on attachment 482712 [details] [diff] [review]
And this should hopefully be the final version.

Looks good.

Comment 7

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

Pushed as http://hg.mozilla.org/comm-central/rev/14c8a06940d6