Remove xpidl [array] use in calIRecurrenceRule.idl
Categories
(Calendar :: General, task)
Tracking
(Not tracked)
People
(Reporter: khushil324, Assigned: benc)
References
Details
Attachments
(1 file, 1 obsolete file)
33.61 KB,
patch
|
benc
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•4 years ago
|
||
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.
Updated•4 years ago
|
Updated•4 years ago
|
Comment 2•4 years ago
|
||
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.
Assignee | ||
Comment 3•4 years ago
|
||
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
Assignee | ||
Updated•4 years ago
|
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/e71712f49a91
Remove xpidl [array] use in calIRecurrenceRule. r=darktrojan
Updated•4 years ago
|
Description
•