Closed Bug 1647756 Opened 4 years ago Closed 4 years ago

Clarify CalDAV LegacySAXResponse code

Categories

(Calendar :: Internal Components, task, P3)

Product:
Calendar
Component:
Internal Components
Type:
task

Tracking

(thunderbird_esr78 fixed, thunderbird79 fixed)

Status:
RESOLVED FIXED
Milestone:
Thunderbird 80.0
Tracking Flags:
Tracking Status
thunderbird_esr78 --- fixed
thunderbird79 --- fixed

People

(Reporter: pmorris, Assigned: pmorris)

References

Details

Attachments

(1 file)

clarify-legacy-sax-response-0.diff
(deleted), patch
darktrojan
: review+
darktrojan
: approval-calendar-beta+
rjl
: approval-comm-esr78+
Details | Diff | Splinter Review

While reviewing bug 1647657 I thought of some ways to clarify the code in LegacySAXResponse. A patch is on the way.

Attachment #9158578 - Flags: review?(geoff)
Attachment #9158578 - Flags: approval-calendar-beta?(geoff)
Attachment #9158578 - Flags: review?(geoff)
Attachment #9158578 - Flags: review+
Attachment #9158578 - Flags: approval-calendar-beta?(geoff)
Attachment #9158578 - Flags: approval-calendar-beta+

The patch for this bug got review+ over three weeks ago and also got approval beta, but it has not landed anywhere. Paul, is this ready to land?

Flags: needinfo?(paul)

Thanks for catching this Martin, this is indeed ready to land. I just forgot to set the checkin-needed-tb flag.

Flags: needinfo?(paul)
Keywords: checkin-needed-tb

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/708f0c5465d0
Clarify CalDAV LegacySAXResponse code. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 80.0
Comment on attachment 9158578 [details] [diff] [review] clarify-legacy-sax-response-0.diff [Approval Request Comment] This patch was originally approved for beta on 6/23 at which time 78 was in beta. (I just forgot to add the checkin-needed-tb flag at that time.) So this should ideally get uplifted to esr78. It is just some minor refactoring to make this code easier to understand. It would be helpful to make any future uplifts smoother. Regression caused by (bug #): no regression. User impact if declined: no user impact if declined Risk to taking this patch (and alternatives if risky): very little risk
Attachment #9158578 - Flags: approval-comm-esr78?
Comment on attachment 9158578 [details] [diff] [review] clarify-legacy-sax-response-0.diff Well, that should have been approval-calendar-esr, still for the 78 ESR.
Attachment #9158578 - Flags: approval-comm-esr78? → approval-calendar-esr?(philipp)
Comment on attachment 9158578 [details] [diff] [review] clarify-legacy-sax-response-0.diff [Triage Comment] Nope you were right the first time for uplift to 78.
Attachment #9158578 - Flags: approval-calendar-esr?(philipp) → approval-comm-esr78+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: