1557504 - remove [array] use in xpidl from calendar/

Status: RESOLVED FIXED

(Calendar :: General, task, P2)

Description

[Magnus Melin [:mkmelin]]

Per bug 1551704, replace usage of [array] from calendar/

These:

• https://searchfox.org/comm-central/search?q=retval%2C+array%2C+size_is&case=false&regexp=false&path=calendar
• https://searchfox.org/comm-central/search?q=array%2C+size_is(aCount)%2C+retval&case=false&regexp=false&path=calendar
• https://searchfox.org/comm-central/search?q=%5Barray%2C+size_is(aCount)%5D&case=false&regexp=false&path=calendar

Might be best with one patch per interface.

Comment 1

[Benjamin Flanagin]

Rodger that

Comment 2

[Magnus Melin [:mkmelin]]

Probably best to do one patch per interface.

Comment 3

[Benjamin Flanagin]

example at
https://bugzilla.mozilla.org/show_bug.cgi?id=1562152

Comment 4

[Benjamin Flanagin]

Attachment: bug-1557504-callImportExport.idl

one sec I'm a goof

Comment 5

[Benjamin Flanagin]

Attachment: bug-1557504-callImportExport.patch

Renamed the patch

Comment 6

[Benjamin Flanagin]

Attachment: bug-1557504-calIItipItem.patch

Second patch, for calIItipItem.idl

Comment 7

[Benjamin Flanagin]

Attachment: bug-1557504-calCalendarACLManager.patch

Comment 8

[Benjamin Flanagin]

Attachment: bug-1557504-calCalendarSearchProvider.patch

Comment 9

[Benjamin Flanagin]

Attachment: bug-1557504-calItemBase.patch

Comment 10

[Benjamin Flanagin]

Attachment: bug-1557504-calItemBase_js.patch

Comment 11

[Benjamin Flanagin]

Attachment: bug-1557504-callPrintFormatter.patch

Comment 12

[Benjamin Flanagin]

Attachment: bug-1557504-calIcsSerializer.patch

Comment 13

[Benjamin Flanagin]

Attachment: bug-1557504-calCalendarViewController.patch

Comment 14

[Benjamin Flanagin]

I think that's all of them.

Comment 15

[Richard Marti (:Paenglab)]

Benjamin, you should ask for review. Like it is now, nothing will happen.

Comment 16

[Magnus Melin [:mkmelin]]

You should not just change the idl, you also need to go through all the callers of the functions you change.

Comment 17

[Magnus Melin [:mkmelin]]

Also each patch needs to have the proper commit message properly formatted.
I'd suggest doing one or two interfaces first completely and then continue on with the next.

Comment 18

[Magnus Melin [:mkmelin]]

Ben, please take over here since Benjamin is no longer working for Thunderbird.

Comment 19

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIImportExport-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b99e5ab6405c591f250d6f89225ae59bfd206729

Comment 20

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIItipItem-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5e1a669cda01722272457b8600ff350772a9f42e

Comment 21

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIItemBase-calIAlarm-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9cb5c0e7b0e90d36df874f00e92e6254ef622c07

Comment 22

[Paul Morris [:pmorris]]

Comment on attachment 9108469 [details] [diff] [review]
Bug-1557504_remove-array-calIItemBase-calIAlarm-0.patch

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

Changes look good.  I haven't tested, but the try run covers that in this case.

Comment 23

[Khushil Mistry [:khushil324]]

calIAlarm.idl
calICalendar.idl
calICalendarACLManager.idl
calICalendarManager.idl
calICalendarSearchProvider.idl
calICalendarView.idl
calICalendarViewController.idl
calIChangeLog.idl
calIDecoratedView.idl
calIIcsParser.idl
calIIcsSerializer.idl
calIICSService.idl
calIImportExport.idl
calIItemBase.idl
calIItipItem.idl
calIItipTransport.idl
calIPrintFormatter.idl
calIRecurrenceInfo.idl
calIRecurrenceItem.idl
calIRecurrenceRule.idl

Comment 24

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarManager-calICalendar-0.patch

Comment 25

[Khushil Mistry [:khushil324]]

Comment on attachment 9108524 [details] [diff] [review]
Bug-1557504_remove-array-calICalendarManager-calICalendar-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=3c1d257449956b1a7c2689e7dc76c4ddb6b497f0

Comment 26

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIPrintFormatter-calIItipTransport-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b07364a3f955a9b0eaeba5c14285a8288583c4a9

Comment 27

[Jorg K (CEST = GMT+2)]

Can you tell me what you want checked in? Attachment 9108469 [details] [diff] which is the only one with r+ so far? Have you looked at your own try run?
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9cb5c0e7b0e90d36df874f00e92e6254ef622c07
That looks pretty terrible to me. Firstly, you haven't rebased for a while, so you're getting errors you shouldn't be getting and second, the mochitests (bct) which include the calendar tests are pretty bad, for example:
TEST-UNEXPECTED-FAIL | comm/calendar/test/browser/preferences/browser_categoryColors.js | Uncaught exception - at resource://testing-common/mozmill

Also, for try runs, just do Linux and Mac opt, since you're messing with JS only, we don't need debug builds and we don't need Windows either.

Comment 28

[Jorg K (CEST = GMT+2)]

Comment on attachment 9108469 [details] [diff] [review]
Bug-1557504_remove-array-calIItemBase-calIAlarm-0.patch

Test failures :-(

Comment 29

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarSearchProvider-calICalendarViewController-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a3eca106c8fb95551cec58325d2bd2c38fe40c77

ESLint error fixed.

Comment 30

[Khushil Mistry [:khushil324]]

(In reply to Jorg K (GMT+2) from comment #28)

Test failures :-(

Let me update the patch and submit it again.

Comment 31

[Jorg K (CEST = GMT+2)]

Thanks, that last try run looks good:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b60ff50c2e289db4e7613a0f8f0bc31d0f536918

Comment 32

[Paul Morris [:pmorris]]

Comment on attachment 9108316 [details] [diff] [review]
Bug-1557504_remove-array-calIImportExport-0.patch

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

Changes look good overall.  I have one question though.

::: calendar/import-export/calIcsImportExport.js
@@ +35,3 @@
>      let parser = Cc["@mozilla.org/calendar/ics-parser;1"].createInstance(Ci.calIIcsParser);
>      parser.parseFromStream(aStream, null);
> +    return parser.getItems({});

Hm, can you explain what the story is with this getItems call?  Looks like it takes an integer not an object ("uint32_t").
https://searchfox.org/comm-central/source/calendar/base/public/calIIcsParser.idl#77

Comment 33

[Paul Morris [:pmorris]]

Comment on attachment 9108342 [details] [diff] [review]
Bug-1557504_remove-array-calIItipItem-0.patch

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

Changes look good. r+

Comment 34

[Khushil Mistry [:khushil324]]

(In reply to Paul Morris [:pmorris] from comment #32)

Hm, can you explain what the story is with this getItems call? Looks like
it takes an integer not an object ("uint32_t").
https://searchfox.org/comm-central/source/calendar/base/public/calIIcsParser.
idl#77

It will be used like this: https://searchfox.org/comm-central/source/calendar/base/src/calIcsParser.js#170
And anyway, it will get removed and the related patch is also submitted.

Comment 35

[Jorg K (CEST = GMT+2)]

Khushil, can you attach the new version of Bug-1557504_remove-array-calIItemBase-calIAlarm-0.patch. Looks like that can land together with Bug-1557504_remove-array-calIItipItem-0.patch.

Comment 36

[Khushil Mistry [:khushil324]]

Yes, wait. There are three patches that can go together. I am just updating them in sequence.

Comment 37

[Paul Morris [:pmorris]]

Comment on attachment 9108524 [details] [diff] [review]
Bug-1557504_remove-array-calICalendarManager-calICalendar-0.patch

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

Looks good overall.  A couple of questions / things to address, and we should check with Magnus about the aArgs thing.

::: calendar/providers/composite/calCompositeCalendar.js
@@ +512,5 @@
>      if (this.mMaxItems && this.mItemsReceived >= this.mMaxItems) {
>        return;
>      }
>  
> +    let aCount = aItems.length;

"aCount" is not the name you need here.  The "a" is to indicate an argument passed to the function.  You could use "count" but that's vague, so I'd go with "itemsCount" or "itemsLength", or maybe best is just skip the separate variable and use "aItems.length".

::: calendar/providers/ics/calICSCalendar.js
@@ +366,5 @@
>            self.forceRefresh();
>          }
>        },
> +      onGetResult: function(aCalendar, aStatus, aItemType, aDetail, aItems) {
> +        this.serializer.addItems(aItems, {});

Instead of `{}`, seems like we'd want `aItems.length` for the second argument here?

@@ +528,5 @@
>        this.mAction = action;
>      }
>      modListener.prototype = {
>        QueryInterface: ChromeUtils.generateQI([Ci.calIOperationListener]),
> +      onGetResult: function(aCalendar, aStatus, aItemType, aDetail, aItems) {},

Based on Magnus' code reviews I've been avoiding using the aArgs naming convention for function arguments.  (See bug 1533520.)  So ideally we'd avoid them in this patch, and move away from them where we are making changes to function arguments.  That said, this is already such a big set of changes, and you've already done them, so I'm not sure if we want to add that to it now.  We should ask Magnus.

Comment 38

[Paul Morris [:pmorris]]

Comment on attachment 9108316 [details] [diff] [review]
Bug-1557504_remove-array-calIImportExport-0.patch

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

r+ Thanks for the explanation.

Comment 39

[Paul Morris [:pmorris]]

Comment on attachment 9108533 [details] [diff] [review]
Bug-1557504_remove-array-calIPrintFormatter-calIItipTransport-0.patch

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

Changes look good.

::: calendar/import-export/calListFormatter.js
@@ +23,4 @@
>      let htmlexporter = Cc["@mozilla.org/calendar/export;1?type=htmllist"].createInstance(
>        Ci.calIExporter
>      );
> +    htmlexporter.exportToStream(aStream, {}, aItems, aTitle);

I take it the `{}` instead of `aCount` here is similar to those in the other patch, where it doesn't actually need to be a number, and possibly will be removed by another patch.  Looks like this is the interface for it:
https://searchfox.org/comm-central/source/calendar/base/public/calIImportExport.idl#61

Comment 40

[Paul Morris [:pmorris]]

Comment on attachment 9108641 [details] [diff] [review]
Bug-1557504_remove-array-calICalendarSearchProvider-calICalendarViewController-0.patch

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

Changes look good here too.  Will be great to have all of these updated to the newer/better approach!

Comment 41

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIPrintFormatter-calIItipTransport-1.patch

Comment 42

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIItemBase-calIAlarm-1.patch

Comment 43

[Khushil Mistry [:khushil324]]

Order:
Bug-1557504_remove-array-calIImportExport.patch(base)
Bug-1557504_remove-array-calIItipItem.patch
Bug-1557504_remove-array-calIItemBase-calIAlarm.patch
Bug-1557504_remove-array-calIPrintFormatter-calIItipTransport.patch
Bug-1557504_remove-array-calICalendarSearchProvider-calICalendarViewController.patch(tip)

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1a988642b3dfb70c9b2696a81c74bf28dc12424f
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=30acc4bc0e80c467000de9774801f1bdad396624

Comment 44

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarSearchProvider-calICalendarViewController-1.patch

Fixes for ESLint error.

Comment 45

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1ce2ab429d0b
remove [array] use in xpidl from calIImportExport.idl. r=pmorris
https://hg.mozilla.org/comm-central/rev/64e5700c2428
remove [array] use in xpidl from calIItipItem.idl. r=pmorris
https://hg.mozilla.org/comm-central/rev/110ee34fe561
remove [array] use in xpidl from calIItemBase.idl and calIAlarm.idl. r=pmorris

Comment 46

[Jorg K (CEST = GMT+2)]

I'll land
Bug-1557504_remove-array-calIPrintFormatter-calIItipTransport.patch
Bug-1557504_remove-array-calICalendarSearchProvider-calICalendarViewController.patch(tip)
later.

Comment 47

[Jorg K (CEST = GMT+2)]

Hmm, https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1a988642b3dfb70c9b2696a81c74bf28dc12424f didn't run any tests. Here's a new one:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=8ed1374ecf945e2eeab6259a362ddf572c0b33a0

Comment 48

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/bfb3d45e856f
remove [array] use in xpidl from calIItipTransport.idl and calIPrintFormatter.idl. r=pmorris
https://hg.mozilla.org/comm-central/rev/d876748e86f5
remove [array] use in xpidl from calICalendarSearchProvider.idl and calICalendarViewController.idl. r=pmorris

Comment 49

[Magnus Melin [:mkmelin]]

Comment on attachment 9108524 [details] [diff] [review]
Bug-1557504_remove-array-calICalendarManager-calICalendar-0.patch

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

::: calendar/providers/ics/calICSCalendar.js
@@ +366,5 @@
>            self.forceRefresh();
>          }
>        },
> +      onGetResult: function(aCalendar, aStatus, aItemType, aDetail, aItems) {
> +        this.serializer.addItems(aItems, {});

I think there should be no second parameter, once calIIcsSerializer  is changed. But it's not so far, so seems like a wrong change to me

@@ +528,5 @@
>        this.mAction = action;
>      }
>      modListener.prototype = {
>        QueryInterface: ChromeUtils.generateQI([Ci.calIOperationListener]),
> +      onGetResult: function(aCalendar, aStatus, aItemType, aDetail, aItems) {},

I think we can keep aNaming changes out of this patch.

Comment 50

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarManager-calICalendar-1.patch

Comment 51

[Paul Morris [:pmorris]]

Comment on attachment 9109116 [details] [diff] [review]
Bug-1557504_remove-array-calICalendarManager-calICalendar-1.patch

Hm, I have no idea why, but there's no "diff" or "splinter review" link for this patch.  I did a manual diff of the two patches.  

I see some aArg conversions which seems fine to me.  Also some functions have the args (like {}) removed that we're removing in this bug.  I assume that's because other patches in this bug have now landed, so we're updating these calls to match.  

For the things in my previous review, the aCount -> itemCount thing looks good.   On `this.serializer.addItems(aItems, {});` it's unchanged from earlier patch, and Magnus made a comment about it in comment 49.  r+ if you can help me understand what the story is for that second argument there.

Comment 52

[Paul Morris [:pmorris]]

Comment on attachment 9109116 [details] [diff] [review]
Bug-1557504_remove-array-calICalendarManager-calICalendar-1.patch

Oh, I see the checkbox for "patch" is not checked for this patch.  Let's see if checking it gets the diff and review links to appear.
Edit: yep, that worked.

Comment 53

[Jorg K (CEST = GMT+2)]

So this is the last patch that needs to land here? Bug-1557504_remove-array-calICalendarManager-calICalendar-1.patch

Any try run?

Comment 54

[Khushil Mistry [:khushil324]]

(In reply to Jorg K (GMT+2) from comment #53)

So this is the last patch that needs to land here? Bug-1557504_remove-array-calICalendarManager-calICalendar-1.patch
No, there are 10 more files to take care.

Any try run?

No try runs yet.

Comment 55

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarManager-calICalendar-2.patch

Updated patch with addItems arguement change.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=aaa13d5df674202dfd24e23397c242d3f82d6a8e

Comment 56

[Jorg K (CEST = GMT+2)]

Hmm, linting error.

Comment 57

[Jorg K (CEST = GMT+2)]

Umm, what about the linting error?

Comment 58

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarManager-calICalendar-3.patch

Comment 59

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/2a0f5703a010
remove [array] use in xpidl from calICalendarManager.idl and calICalendar.idl. r=pmorris

Comment 60

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarACLManager-calICalendarView-calIDecoratedView-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f7bdbbf6015ec42c6649166dc4047759a1c1322a

Comment 61

[Paul Morris [:pmorris]]

Comment on attachment 9109931 [details] [diff] [review]
Bug-1557504_remove-array-calICalendarACLManager-calICalendarView-calIDecoratedView-0.patch

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

Changes look good.  For future reference, in cases like this, you could even just change one function per patch.  That makes review simpler, keeps similar changes together in the history, etc.

Comment 62

[Jorg K (CEST = GMT+2)]

Please rebase Bug-1557504_remove-array-calICalendarACLManager-calICalendarView-calIDecoratedView-0.patch and add a bug number to the commit message:
$ hg qpush
applying Bug-1557504_remove-array-calICalendarACLManager-calICalendarView-calIDecoratedView.patch
patching file calendar/base/content/calendar-views-utils.js
Hunk #3 FAILED at 531
1 out of 3 hunks FAILED -- saving rejects to file calendar/base/content/calendar-views-utils.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh Bug-1557504_remove-array-calICalendarACLManager-calICalendarView-calIDecoratedView.patch

Maybe you'll need to wait until my push is done to see the conflict.

Comment 63

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarACLManager-calICalendarView-calIDecoratedView-1.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central

Comment 64

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calICalendarACLManager-calICalendarView-calIDecoratedView-2.patch

Updated the commit message.

Comment 65

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIChangeLog-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=983b9bf16bbb5d3bb7cb373892b2bb7f065a63db

Comment 66

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIIcsSerializer-0.patch

Comment 67

[Khushil Mistry [:khushil324]]

Comment on attachment 9110289 [details] [diff] [review]
Bug-1557504_remove-array-calIIcsSerializer-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6136681cfe465d216e881e46185158f4af5e9890

Comment 68

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIIcsParser-calIICSService-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=1a42b861ec6fb6f1f914b99029c36f5a74603547

Comment 69

[Paul Morris [:pmorris]]

Comment on attachment 9110284 [details] [diff] [review]
Bug-1557504_remove-array-calIChangeLog-0.patch

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

Looks good.  A few minor comments to address.

::: calendar/providers/caldav/calDavCalendar.js
@@ +302,5 @@
>  
>    fetchCachedMetaData: function() {
>      cal.LOG("CalDAV: Retrieving server info from cache for " + this.name);
> +    let cacheIds = this.mOfflineStorage.getAllMetaDataIds();
> +    let cacheValues = this.mOfflineStorage.getAllMetaDataValues();

Ahhhh, so much nicer!

::: calendar/providers/storage/calStorageCalendar.js
@@ +2516,2 @@
>      } catch (e) {
>        this.logError("Error getting all metadata!", e);

The error messages here and below need to be updated to match the changed functions.

@@ +2534,5 @@
> +    } finally {
> +      query.reset();
> +    }
> +    return values;
> +  },

Since these two functions are basically the same except for the id/value parts, you could merge them into a single "internal" function, something like this:

_getAllMetaDataResults: function(key) {
    let query = this.mSelectAllMetaData;
    let results = [];
    try {
      this.prepareStatement(query);
      while (query.executeStep()) {
        results.push(query.row[key]);
      }
    } catch (e) {
      this.logError(`Error getting all metadata ${key == "item_id" ? "IDs" : "values"} ` + , e);
    } finally {
      query.reset();
    }
    return results;
  }
getAllMetaDataIds: this._getAllMetaDataResults.bind(null, "item_id"),
getAllMetaDataValues: this._getAllMetaDataResults.bind(null, "value"),

::: calendar/test/unit/test_providers.js
@@ +337,5 @@
>      equal(aCalendar.getMetaData("item2"), "meta2");
>  
> +    let ids, values;
> +    ids = aCalendar.getAllMetaDataIds();
> +    values = aCalendar.getAllMetaDataValues();

I would just do:

let ids = ...
let values = ...

No need for separate let declaration.

@@ +338,5 @@
>  
> +    let ids, values;
> +    ids = aCalendar.getAllMetaDataIds();
> +    values = aCalendar.getAllMetaDataValues();
> +    equal(values.length, 2);

For good measure, I'd also add: 
equal(ids.length, 2);
Here and in similar cases below.

@@ +360,5 @@
>  
>      aCalendar.deleteMetaData("item2");
>      equal(aCalendar.getMetaData("item2"), null);
> +    values = aCalendar.getAllMetaDataValues();
> +    equal(values.length, 0);

Might as well check ids as well, here and below.

Comment 70

[Paul Morris [:pmorris]]

Comment on attachment 9110289 [details] [diff] [review]
Bug-1557504_remove-array-calIIcsSerializer-0.patch

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

LGTM from inspecting the changes.

Comment 71

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8dc56944ca5a
remove [array] use in xpidl from calICalendarACLManager.idl, calICalendarView.idl and calIDecoratedView.idl. r=pmorris
https://hg.mozilla.org/comm-central/rev/dd38ba65ea98
remove [array] use in xpidl from calIIcsSerializer.idl. r=pmorris

Comment 72

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIChangeLog-1.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=628ec4a685177f4eaaec1b54a519c8b147a15fa0

Comment 73

[Paul Morris [:pmorris]]

Comment on attachment 9110294 [details] [diff] [review]
Bug-1557504_remove-array-calIIcsParser-calIICSService-0.patch

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

LGTM from reading the changes.

Comment 74

[Paul Morris [:pmorris]]

Comment on attachment 9110569 [details] [diff] [review]
Bug-1557504_remove-array-calIChangeLog-1.patch

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

Looks good, thanks. r+

Comment 75

[Geoff Lankow (:darktrojan)]

The Try runs for both of the latest patches went wrong. More work to be done.

Comment 76

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIChangeLog-2.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=aa6077ae255cf28ef124b045155c39911144f82d

Comment 77

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4f4eb9541c61
remove [array] use in xpidl from calIChangeLog.idl. r=pmorris

Comment 78

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIIcsParser-1.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=2dbe1c6be1bfc90b7b6ac1e4fed60eb7755e2bb3

Comment 79

[Khushil Mistry [:khushil324]]

Attachment: Bug-1557504_remove-array-calIWcapCalendar-0.patch

Comment 80

[Khushil Mistry [:khushil324]]

Comment on attachment 9111364 [details] [diff] [review]
Bug-1557504_remove-array-calIWcapCalendar-0.patch

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c975254f24c2f9312a8b257bc20aaf121ecb7061

Comment 81

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8ec25287b881
remove [array] use in xpidl from calIIcsParser.idl. r=pmorris

Comment 82

[Paul Morris [:pmorris]]

Comment on attachment 9111364 [details] [diff] [review]
Bug-1557504_remove-array-calIWcapCalendar-0.patch

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

LGTM, from reading the changes.

Comment 83

[Geoff Lankow (:darktrojan)]

Khushil please update your comm-central tree to a known good revision before doing a Try run. We shouldn't have to sort through the failures to work out which ones might have been caused by the patch being tested.

Comment 84

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cffc48bec697
remove [array] use in xpidl from calIWcapCalendar.idl. r=pmorris DONTBUILD

Comment 85

[Magnus Melin [:mkmelin]]

Looks like done now,