1458367 - Mass replace the Components.interface/Components.utils uses with Ci/Cu in Calendar

Status: RESOLVED FIXED

(Calendar :: General, defect, P3)

Description

[Jorg K (CEST = GMT+2)]

+++ This bug was initially created as a clone of Bug #1436605 +++

In bug 1436605 were replaced everything in C-C apart from Calendar. Calendar was doing some refactoring at the time, the replacement will have to happen there in due course.

Comment 1

[Geoff Lankow (:darktrojan)]

Please wait until bug 1469459 is fixed as it'll save time cleaning up bitrot.

Comment 2

[Martin Schröder [:mschroeder]]

(In reply to Geoff Lankow (:darktrojan) from comment #1)
> Please wait until bug 1469459 is fixed as it'll save time cleaning up bitrot.

Oh, would it have been filed in the correct "Product"...

Comment 3

[Jorg K (CEST = GMT+2)]

OK, when you're done here, I'd finally like to fix all spelling mistakes in Calendar, there are heaps! Prior attempt, never landed, see attachment 8960495 [details] [diff] [review].

Comment 4

[Geoff Lankow (:darktrojan)]

Is this likely to happen soon, Martin? If you'd like me to do it instead I can.

Comment 5

[Martin Schröder [:mschroeder]]

The patch is already prepared, I just have to fix some cosmetic things. I will try to wrap it up tonight.

Comment 6

[Martin Schröder [:mschroeder]]

Attachment: Patch v1

Comment 7

[[:MakeMyDay]]

Comment on attachment 9011254 [details] [diff] [review]
Patch v1

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

Thank you for this conversion. Looks like you got your build environment back :-)

r+wc - mainly nits, but for the gdata code, we need a polyfill of the new shortcuts with the previous notion if not available to retain backwards compatibility.

One thing I haven't commented on is the line length of some of the touched code. We currently haven't the linter rule for that enabled, but it would be apprciated if we don't exceed a line length of 100 characters when touching code.

When you fixed everything for checkin, a try run would be nice before landing, just to be on the safe side.

::: calendar/.eslintrc.js
@@ +476,5 @@
>  
>          // Enforce consistent indentation (4-space)
>          "indent-legacy": [2, 4, { SwitchCase: 1, }],
>  
> +        "mozilla/use-cc-etc": 2,

Please add a comment to explain the scope of the rule

::: calendar/base/src/calAlarmMonitor.js
@@ +29,1 @@
>  ];

Can you fold this into one line?

::: calendar/base/src/calAlarmService.js
@@ +103,1 @@
>  ];

Same here

::: calendar/base/src/calCalendarManager.js
@@ +799,1 @@
>      ]),

Here again.

::: calendar/base/src/calRecurrenceDate.js
@@ +15,1 @@
>  ];

Here as well

::: calendar/providers/caldav/calDavCalendar.js
@@ +75,5 @@
>  // some shorthand
> +var calICalendar = Ci.calICalendar;
> +var calIErrors = Ci.calIErrors;
> +var calIFreeBusyInterval = Ci.calIFreeBusyInterval;
> +var calICalDavCalendar = Ci.calICalDavCalendar;

Since the var name are not really shorter here anymore, we could get rid of the extra var alltogether here.

::: calendar/providers/gdata/components/calGoogleCalendar.js
@@ +26,5 @@
>      migrateItemMetadata,
>      JSONToAlarm
>  } = ChromeUtils.import("resource://gdata-provider/modules/gdataUtils.jsm", null);
>  
> +var cIOL = Ci.calIOperationListener;

The Provider for Google needs probably a special treatment since this change is not backward compatible, the provider is build from c-c and currently supports versions back to 45 (even if that would be raised to 52, that problem still would exist).

Can you add a Ci/Cc/Cu/Cr (as applicable) var in the files if these are not already available globally to make that change backwards compatible? Please add a comment that this special handling could be removed once the minimum requirement is raised to TB60.

@@ +104,1 @@
>              throw new Components.Exception("", cIE.CAL_IS_READONLY);

No need to assign a cont here.

::: calendar/providers/wcap/public/calIWcapCalendar.idl
@@ +252,4 @@
>  //     calIOperation defineAccessControl(
>  //         in string userId, in unsigned long accessControlBits,
>  //         in calIGenericOperationListener listener);
>      

whitespce

Comment 8

[Jorg K (CEST = GMT+2)]

Stalled for two months now (and likely with bit rot)?

Comment 9

[Jorg K (CEST = GMT+2)]

(In reply to Geoff Lankow (:darktrojan) from comment #4)

Is this likely to happen soon, Martin? If you'd like me to do it instead I
can.

How much longer do we want to wait here?

Comment 10

[Geoff Lankow (:darktrojan)]

Attachment: 1458367-use-cc-etc-1.diff

I've updated the patch and addressed the review comments, but this probably needs reviewing again given how much time has passed.

Comment 11

[Jorg K (CEST = GMT+2)]

You think MMD wants to look at 700 KB again? That's torture. Do a try run and ship it.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4ae0c5713dfe
Mass replace the Components.interface/Components.utils uses with Ci/Cu in Calendar. r=MakeMyDay