remove [array] use in xpidl from calendar/
Categories
(Calendar :: General, task, P2)
Tracking
(Not tracked)
People
(Reporter: mkmelin, Assigned: khushil324)
References
Details
Attachments
(12 files, 20 obsolete files)
(deleted),
patch
|
pmorris
:
review+
mkmelin
:
feedback+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
pmorris
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
pmorris
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
pmorris
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
khushil324
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
pmorris
:
review+
|
Details | Diff | Splinter Review |
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®exp=false&path=calendar
- https://searchfox.org/comm-central/search?q=array%2C+size_is(aCount)%2C+retval&case=false®exp=false&path=calendar
- https://searchfox.org/comm-central/search?q=%5Barray%2C+size_is(aCount)%5D&case=false®exp=false&path=calendar
Might be best with one patch per interface.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Rodger that
Reporter | ||
Comment 2•5 years ago
|
||
Probably best to do one patch per interface.
Comment 3•5 years ago
|
||
Comment 5•5 years ago
|
||
Renamed the patch
Comment 6•5 years ago
|
||
Second patch, for calIItipItem.idl
Comment 7•5 years ago
|
||
Comment 8•5 years ago
|
||
Comment 9•5 years ago
|
||
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
Comment 12•5 years ago
|
||
Comment 13•5 years ago
|
||
Comment 14•5 years ago
|
||
I think that's all of them.
Updated•5 years ago
|
Comment 15•5 years ago
|
||
Benjamin, you should ask for review. Like it is now, nothing will happen.
Reporter | ||
Comment 16•5 years ago
|
||
You should not just change the idl, you also need to go through all the callers of the functions you change.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 17•5 years ago
|
||
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.
Reporter | ||
Comment 18•5 years ago
|
||
Ben, please take over here since Benjamin is no longer working for Thunderbird.
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Comment 19•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 20•5 years ago
|
||
Assignee | ||
Comment 21•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 22•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 23•5 years ago
|
||
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
Assignee | ||
Comment 24•5 years ago
|
||
Assignee | ||
Comment 25•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 26•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Comment 27•5 years ago
|
||
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•5 years ago
|
||
Assignee | ||
Comment 30•5 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #28)
Test failures :-(
Let me update the patch and submit it again.
Comment 31•5 years ago
|
||
Thanks, that last try run looks good:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=b60ff50c2e289db4e7613a0f8f0bc31d0f536918
Reporter | ||
Updated•5 years ago
|
Comment 32•5 years ago
|
||
Comment 33•5 years ago
|
||
Assignee | ||
Comment 34•5 years ago
|
||
(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•5 years ago
|
||
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.
Assignee | ||
Comment 36•5 years ago
|
||
Yes, wait. There are three patches that can go together. I am just updating them in sequence.
Comment 37•5 years ago
|
||
Comment 38•5 years ago
|
||
Comment 39•5 years ago
|
||
Comment 40•5 years ago
|
||
Assignee | ||
Comment 41•5 years ago
|
||
Assignee | ||
Comment 42•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 43•5 years ago
|
||
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
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 44•5 years ago
|
||
Fixes for ESLint error.
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 45•5 years ago
|
||
Pushed by mozilla@jorgk.com:
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
Updated•5 years ago
|
Comment 46•5 years ago
|
||
I'll land
Bug-1557504_remove-array-calIPrintFormatter-calIItipTransport.patch
Bug-1557504_remove-array-calICalendarSearchProvider-calICalendarViewController.patch(tip)
later.
Updated•5 years ago
|
Comment 47•5 years ago
|
||
Comment 48•5 years ago
|
||
Pushed by mozilla@jorgk.com:
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
Reporter | ||
Comment 49•5 years ago
|
||
Assignee | ||
Comment 50•5 years ago
|
||
Comment 51•5 years ago
|
||
Comment 52•5 years ago
|
||
Comment 53•5 years ago
|
||
So this is the last patch that needs to land here? Bug-1557504_remove-array-calICalendarManager-calICalendar-1.patch
Any try run?
Assignee | ||
Comment 54•5 years ago
|
||
(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.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 55•5 years ago
|
||
Updated patch with addItems arguement change.
Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=aaa13d5df674202dfd24e23397c242d3f82d6a8e
Assignee | ||
Updated•5 years ago
|
Comment 56•5 years ago
|
||
Hmm, linting error.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 58•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 59•5 years ago
|
||
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/2a0f5703a010
remove [array] use in xpidl from calICalendarManager.idl and calICalendar.idl. r=pmorris
Assignee | ||
Comment 60•5 years ago
|
||
Updated•5 years ago
|
Comment 61•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 62•5 years ago
|
||
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.
Assignee | ||
Comment 63•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 64•5 years ago
|
||
Updated the commit message.
Assignee | ||
Comment 65•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 66•5 years ago
|
||
Assignee | ||
Comment 67•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 68•5 years ago
|
||
Comment 69•5 years ago
|
||
Comment 70•5 years ago
|
||
Comment 71•5 years ago
|
||
Pushed by geoff@darktrojan.net:
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
Assignee | ||
Comment 72•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 73•5 years ago
|
||
Comment 74•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 75•5 years ago
|
||
The Try runs for both of the latest patches went wrong. More work to be done.
Assignee | ||
Comment 76•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Updated•5 years ago
|
Comment 77•5 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/4f4eb9541c61
remove [array] use in xpidl from calIChangeLog.idl. r=pmorris
Assignee | ||
Comment 78•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 79•5 years ago
|
||
Assignee | ||
Comment 80•5 years ago
|
||
Comment 81•5 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/8ec25287b881
remove [array] use in xpidl from calIIcsParser.idl. r=pmorris
Comment 82•5 years ago
|
||
Assignee | ||
Updated•5 years ago
|
Comment 83•5 years ago
|
||
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•5 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/cffc48bec697
remove [array] use in xpidl from calIWcapCalendar.idl. r=pmorris DONTBUILD
Reporter | ||
Comment 85•5 years ago
|
||
Looks like done now,
Updated•4 years ago
|
Description
•