Extend eslint coverage to previously preprocessed files

RESOLVED FIXED in 6.4

Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: MakeMyDay, Assigned: MakeMyDay)

Tracking

Lightning 6.2
Dependency tree / graph

Details

Attachments

(1 attachment, 3 obsolete attachments)

Assignee

Description

Last year
calendar/base/content/calendar-dnd-listener.js
calendar/base/content/dialogs/calendar-migration-dialog.js

have been previously preprocessed. since this is not the case anymore, these files should also be subject to linting now.
Assignee

Comment 1

Last year
This patch takes care and apllies on top of patches from bug 393084, 1437405 and 463402 (in that order)
Attachment #8955893 - Flags: review?(philipp)
Assignee

Comment 2

Last year
I borrowed your eslint user for this patch for better transparency when using blame.
Comment on attachment 8955893 [details] [diff] [review]
ExtendEslintCoverageForCalendar-V1.diff

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

This is going to bitrot on my calUtils patches, we should coordinate in what order these get pushed so we can adapt.

::: calendar/base/content/calendar-dnd-listener.js
@@ +320,5 @@
>                          return charset;
>                      },
>  
> +                    onStreamComplete: function(aLoader, context, status, unicharString) {
> +                        let parser = c["@mozilla.org/calendar/ics-parser;1"]

This should probably be Cc?
Attachment #8955893 - Flags: review?(philipp) → review+
Assignee

Comment 4

Last year
Updated version with the comment considered.

Holding back checkin-needed at least until bug 463402 has landed.
Attachment #8955899 - Flags: review+
Is this ready to go?
Flags: needinfo?(makemyday)
Assignee

Comment 6

Last year
If it still applies after the latest calutils consolidation checkin, it is. But I haven't crosschecked.
Flags: needinfo?(makemyday)
Assignee

Updated

Last year
Attachment #8955893 - Attachment is obsolete: true
Assignee

Comment 7

Last year
Debitrotted patch.

Philipp, I leave to your discretion whether to set chekin-needed now or wait until the other patches did land, since it's your patch queue that might get bitrotted. If you postpone it, feel free to debitrot then for checkin.
Attachment #8955899 - Attachment is obsolete: true
Attachment #8964185 - Flags: review+
I updated the patch to apply cleanly.
Attachment #8964185 - Attachment is obsolete: true
Attachment #8971938 - Flags: review+

Comment 9

Last year
Are you sure you want to land such a large patch without any test coverage? The tree rules say: No test, no landing :-(
Please present this again for landing when we have test coverage again, see bug 1457087.
Depends on: 1457087
Keywords: checkin-needed
Calendar Xpcshell tests are back, so let's see how this fares:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5c5504b30dc97a26b93c3194b3d8b025fa2a96fd

OK, looking at the changes, they are mostly cosmetic, so they should break anything, but better safe than sorry ;-)

Comment 13

Last year
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/0698244578e9
Extend eslint coverage for calendar. r=philipp
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED

Updated

Last year
Target Milestone: --- → 6.4
Attachment #8971938 - Flags: approval-calendar-beta?(philipp)
Attachment #8971938 - Flags: approval-calendar-beta?(philipp) → approval-calendar-beta+
Please remove that flag again. This landed on Cal 6.4 currently in beta. There's nothing to uplift.

And, as requested before, grant me permission to set/reset those flags for administrative purposes :-(
Flags: needinfo?(philipp)
Flags: needinfo?(philipp)
Attachment #8971938 - Flags: approval-calendar-beta+
I've given you calendar-drivers access so you can reset flags, but please do not use it to grant approval unless it it test-only.
Thanks, will (not) do. As my aunt said: Use, but don't abuse ;-)
You need to log in before you can comment on or make changes to this bug.