Move calIteratorUtils to cal.iterate namespace

RESOLVED FIXED in 6.2

Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

Lightning 6.2
Dependency tree / graph

Details

Attachments

(2 attachments, 4 obsolete attachments)

calIteratorUtils has a few toplevel cal. functions, and some ical iterators in the cal.ical namespace. I'd like to refactor this to provide all the functions on cal.iterate, and move this under the calUtils submodules loader.
Posted patch Manual Changes - v1 (obsolete) β€” β€” Splinter Review
cal.itemIterator                        -> cal.iterate.items
cal.forEach                             -> cal.iterate.forEach
cal.ical.calendarComponentIterator      -> cal.iterate.icalComponent
cal.ical.subcomponentIterator           -> cal.iterate.icalSubcomponent
cal.ical.propertyIterator               -> cal.iterate.icalProperty
cal.ical.paramIterator                  -> cal.iterate.icalParameter
Attachment #8953815 - Flags: review?(makemyday)
Posted patch Automatic Changes - v1 (obsolete) β€” β€” Splinter Review
Posted patch Manual Changes - v2 (obsolete) β€” β€” Splinter Review
Attachment #8953815 - Attachment is obsolete: true
Attachment #8953815 - Flags: review?(makemyday)
Attachment #8953872 - Flags: review?(makemyday)
Posted patch Automatic Changes - v2 (obsolete) β€” β€” Splinter Review
Attachment #8953816 - Attachment is obsolete: true
Comment on attachment 8953872 [details] [diff] [review]
Manual Changes - v2

Review of attachment 8953872 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r+ with comments considered.

::: calendar/base/modules/calIteratorUtils.jsm
@@ +35,5 @@
> +     * done (and also when cal.forEach.BREAK is returned), calls the completed()
> +     * function if passed.
> +     *
> +     * If you would like to break or continue inside the body(), return either
> +     *     cal.forEach.BREAK or cal.forEach.CONTINUE

The description is still referring to the old cal.forEach.* names.

@@ +48,5 @@
> +     *                          the single item from the iterator.
> +     * @param completed     [optional] The function called after the loop completes.
> +     */
> +    forEach: (() => {
> +        function forEach(iterable, body, completed) {

Can we use different names for the outer and inner forEach here?
Attachment #8953872 - Flags: review?(makemyday) → review+
Posted patch Manual Changes - v3 β€” β€” Splinter Review
(In reply to [:MakeMyDay] from comment #5)

> ::: calendar/base/modules/calIteratorUtils.jsm
> @@ +35,5 @@
> > +     * done (and also when cal.forEach.BREAK is returned), calls the completed()
> > +     * function if passed.
> > +     *
> > +     * If you would like to break or continue inside the body(), return either
> > +     *     cal.forEach.BREAK or cal.forEach.CONTINUE
> 
> The description is still referring to the old cal.forEach.* names.
This is done in the automatic patch, but I only noticed afterwards :) I've also updated various docblocks, and updated some parameters to use default values.


> 
> @@ +48,5 @@
> > +     *                          the single item from the iterator.
> > +     * @param completed     [optional] The function called after the loop completes.
> > +     */
> > +    forEach: (() => {
> > +        function forEach(iterable, body, completed) {
> 
> Can we use different names for the outer and inner forEach here?
I'd rather keep this, to make clear it is meant to be the same function.
Attachment #8953872 - Attachment is obsolete: true
Attachment #8967864 - Flags: review+
Posted patch Automatic Changes - v3 β€” β€” Splinter Review
Attachment #8953873 - Attachment is obsolete: true
Attachment #8967864 - Flags: approval-calendar-beta+
Pushed by mozilla@kewis.ch:
https://hg.mozilla.org/comm-central/rev/3e4b5f228311
Move calIteratorUtils to cal.iterate namespace - manual changes. r=MakeMyDay
https://hg.mozilla.org/comm-central/rev/523ba7b139fe
Move calIteratorUtils to cal.iterate namespace - automatic changes. r=MakeMyDay
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: checkin-needed
Resolution: --- → FIXED
https://hg.mozilla.org/releases/comm-beta/f0dc8dca11f8
Bug 1440982 - Move calIteratorUtils to cal.iterate namespace - automatic changes. r=MakeMyDay
https://hg.mozilla.org/releases/comm-beta/0ef2b2e4c3dd
Bug 1440982 - Move calIteratorUtils to cal.iterate namespace - manual changes. r=MakeMyDay
Target Milestone: --- → 6.2
https://hg.mozilla.org/releases/comm-beta/rev/f0dc8dca11f8
Bug 1440982 - Move calIteratorUtils to cal.iterate namespace - automatic changes. r=MakeMyDay
https://hg.mozilla.org/releases/comm-beta/rev/0ef2b2e4c3dd
Bug 1440982 - Move calIteratorUtils to cal.iterate namespace - manual changes. r=MakeMyDay
You need to log in before you can comment on or make changes to this bug.