If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

trunk calendar link failure looking for calDateTime::PostCreatePrototype

VERIFIED FIXED in 1.0b1

Status

Calendar
Internal Components
--
critical
VERIFIED FIXED
9 years ago
6 years ago

People

(Reporter: dmose, Assigned: dmose)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Created attachment 336681 [details] [diff] [review]
linkage fix, v1

This is the result of some XPConnect changes on trunk.  I haven't actually tested the calendar functionality with the attached patch, but it un-breaks the build, which is a significant step forward.
Attachment #336681 - Flags: review?(mvl)
If you want to commit this so it works for comm-central and leaves cvs alone, then you could change the ifdef to COMM_BUILD, and in the Makefile.in add a section:

ifdef COMM_BUILD
DEFINES += COMM_BUILD
endif

The Sunbird cvs trunk would still compile, as would TB + lightning from comm-central.
Blocks: 449630
Comment on attachment 336681 [details] [diff] [review]
linkage fix, v1

stealing review, r=dbo

You could avoid a warning omitting the (unused) parameter names, but since we're not doing that throughout the whole file and are not compiling with -Wall...

BTW: Anybody knows whether warning-free code is any goal for the mozilla platform? IMO it pays off in better code quality. We've found quite some bugs while making OpenOffice warning-free and meanwhile compile with -Werror to keep it clean.
Attachment #336681 - Flags: review?(mvl) → review+
Personally, I think we always need to start somewhere and should avoid the warning now, even though the rest of the file doesn't do so. Maybe we can make it a goal at least for calendar code.

Comment 4

9 years ago
AFAIK, there is some bug around to eliminate warnings and convert more and more code to using -Werror (some cookie code already does). This also means that compilation usually breaks with every new gcc version, but there seems to be a strong wish to get rid of warnings incrementally across our source.
(In reply to comment #4)
> AFAIK, there is some bug around to eliminate warnings and convert more and more
> code to using -Werror (some cookie code already does). This also means that
Interesting, because this requires the core XPCOM headers and macros to be already warning-free(?)

> compilation usually breaks with every new gcc version, but there seems to be a
> strong wish to get rid of warnings incrementally across our source.
Don't be too pessimistic ;-) My experience doesn't show that, at least w.r.t. gcc3.3/3.4/4.*.
(Assignee)

Comment 6

9 years ago
Created attachment 336909 [details] [diff] [review]
linkage fix, v2

Tweaked as per Standard8's suggestion.
Assignee: nobody → dmose
Attachment #336681 - Attachment is obsolete: true
Status: NEW → ASSIGNED
(Assignee)

Comment 7

9 years ago
In general, I agree with the goal of getting rid of warnings.  W.r.t. this particular patch, does gcc really warn about unused method parameters?  Omitting the params seems like a weird way to make such a warning go away, since it's kind of lying about what the stack looks like.
(Assignee)

Comment 8

9 years ago
Since this is a build-bustage fix, I think going ahead and landing this without going through another review cycle is the right thing to do.
(In reply to comment #7)
> In general, I agree with the goal of getting rid of warnings.  W.r.t. this
> particular patch, does gcc really warn about unused method parameters? 
Yes it does, but depends on the used warning level.

> Omitting the params seems like a weird way to make such a warning go away,
> since it's kind of lying about what the stack looks like.
...but which is irrelevant, because the variables are actually not used. At times I opted to just comment out the names to have them once I need them again.
(Assignee)

Comment 10

9 years ago
Checked in on 1.8 branch and 1.9 trunk; resolving.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Status: RESOLVED → VERIFIED
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.