Closed Bug 1602425 Opened 7 months ago Closed 6 months ago

Remove xpidl [array] use in calIRecurrenceRule.idl

Categories

(Calendar :: General, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 73.0

People

(Reporter: khushil324, Assigned: benc)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → benc
Blocks: 1557504

Mostly straightforward, but I took the opportunity to replace some serious preprocessor macro voodoo with a lookup table, for getComponent()/setComponent(). Hopefully it makes the code easier to read.

No try run yet (I'll do one for all the calendar stuff together), but a local run passes all the unit tests that use getComponent()/setComponent(), so I'm pretty confident it's sound.

Attachment #9117486 - Flags: review?(mkmelin+mozilla)
Attachment #9117486 - Flags: review?(mkmelin+mozilla) → review?(geoff)
Status: NEW → ASSIGNED
Comment on attachment 9117486 [details] [diff] [review]
1602425-xpidl-array-removal-in-calIRecurrenceRule-1.patch

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

Hooray for voodoo reduction. Although I'm sure it didn't look as bad before the formatter got hold of it.

::: calendar/base/backend/libical/calRecurrenceRule.cpp
@@ +261,5 @@
> +                                nsTArray<int16_t> &aValues) {
> +  aValues.ClearAndRetainStorage();
> +  // Look up the array for this component type.
> +  for (int i = 0; recurrenceTable[i].name; ++i) {
> +    auto const &r = recurrenceTable[i];

I'd prefer `r` to be called `row`, assuming that's what it's short for. Same below.
Attachment #9117486 - Flags: review?(geoff) → review+

Renamed r => row.
Good point about the reformat changing the old macro voodoo. I wondered why it was arranged the way it was!

Anyway, a new try build which covers this patch (and the other two calendar ones):
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5cb6d13351199bb8ebbf8a7c2befc2d5c997a16a

Attachment #9117486 - Attachment is obsolete: true
Attachment #9118009 - Flags: review+

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/e71712f49a91
Remove xpidl [array] use in calIRecurrenceRule. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → 73
You need to log in before you can comment on or make changes to this bug.