Closed Bug 1230991 Opened 4 years ago Closed 4 years ago

Remove duplication of theme files in xpi

Categories

(Calendar :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Paenglab, Assigned: Paenglab)

References

Details

Attachments

(1 file, 1 obsolete file)

MakeMyDay gave me a list from build/mozilla/toolkit/mozapps/installer/find-dupes.py with duplicate files in all three platform directories.
Attached patch fileMove.patch (obsolete) — Splinter Review
This patch moves the duplicate files to the shared directory in XPI. I touched only the css files which are the same on all three platforms. Files which are the same in only two platforms are not touched because mostly it is because OS X has different styles.

I also leaved the PNG untouched because they are for the invitation and will probably be in near future removed or changed with the itip refresh.

I also removed calendar/resources/jar.mn because this look never to be touched. I tested this with changing to nonexistant files and never got a error. I tried the same with the moz.build file at the same position and it seems it's also never used. I leaved it there because it's a file for the new build system and I don't know enough if this can really be deleted. If it's used it seems for me not up to date because Linux is not respected in this file.

Or should I remove also this file?
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8696511 - Flags: review?(makemyday)
Comment on attachment 8696511 [details] [diff] [review]
fileMove.patch

Thanks for digging into this.

f+ only, because I'm not sure about removing calendar/resources/jar.mn. Did you build with that patch?

Philipp, are there any objections to remove the jar.mn?
Attachment #8696511 - Flags: review?(philipp)
Attachment #8696511 - Flags: review?(makemyday)
Attachment #8696511 - Flags: feedback+
Comment on attachment 8696511 [details] [diff] [review]
fileMove.patch

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

Nope, fine with me. r=philipp
Attachment #8696511 - Flags: review?(philipp) → review+
Attached patch fileMove.patchSplinter Review
Updated the reviewer in commit message

Philipp, do we want this also in 4.7 for ESR?

And what about the calendar/resources/moz.build, does this need any cleanup? The DEFINES['THEME'] is missing Linux but it seems still working on Linux. Should this be removed, because not used,  or Linux added?

I set checkin-needed. moz.build can be fixed in a new bug.
Attachment #8696511 - Attachment is obsolete: true
Attachment #8700284 - Flags: review+
Attachment #8700284 - Flags: approval-calendar-aurora?(philipp)
Keywords: checkin-needed
Comment on attachment 8700284 [details] [diff] [review]
fileMove.patch

This is not a user visible change, just some backend cleanup, right? In that case I don't think we need to backport this.
Attachment #8700284 - Flags: approval-calendar-aurora?(philipp) → approval-calendar-aurora-
https://hg.mozilla.org/comm-central/rev/7e2cd0643242976e131b495c618be921980287bf
Bug 1230991 - Move common files from platform directories to common. r=Fallen
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.8
Backported to releases/comm-aurora changeset 9f6e321843df
Target Milestone: 4.8 → 4.7
Depends on: 1238405
You need to log in before you can comment on or make changes to this bug.