Closed Bug 759808 Opened 13 years ago Closed 13 years ago

MPL 2 upgrade: Instantbird

Categories

(mozilla.org :: Licensing, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gerv, Assigned: gerv)

References

Details

Attachments

(2 files, 1 obsolete file)

This bug tracks the MPL 2 upgrade for the project named in the subject line. The repo is here: http://hg.instantbird.org/instantbird Gerv
Summary: MPL 2 upgrade: InstantBird → MPL 2 upgrade: Instantbird
Attached patch Patch v.1 (obsolete) (deleted) — Splinter Review
How does this look? Gerv
Attachment #628374 - Flags: review?(florian)
Comment on attachment 628374 [details] [diff] [review] Patch v.1 Files that have been processed but shouldn't IMHO have a license header: instantbird/branding/*/mozicon*.xpm It seems strange to put a license header in icon files, but the format seems to accept it. However, it seems "/* XPM */" should stay at the beginning of the file, like for "<?xml" in XML files. This also affects files that have been touched in mozilla-central and comm-central. instantbird/installer/windows/nsis/updater_append.ini instantbird/locales/updater_append.ini There's a comment at the top of this file stating that it's important that this file starts with a line break, and this file is used to append it to another file that already has a license header, so I think we don't want the header here. The file installer/windows/nsis/updater_append.ini exists for other Mozilla applications in mozilla-central and comm-central, and I think you will want to revert the change to these files. instantbird/themes/smileys/theme.js This file has the .js extension, but it actually contains JSON data, and AFAIK that format doesn't support comments, so no license header there. instantbird/themes/messages/*/Incoming/*.html instantbird/themes/messages/*/NextStatus.html instantbird/themes/messages/*/Status.html tools/messagestyles/teststyles/*/*.html tools/messagestyles/teststyles/*/*/*.html These HTML files are templates that are included for each displayed IM, so license headers there would be wasteful. Wrong comment style for the header: instantbird/locales/l10n.ini The ";" prefix seems wrong here, or at least the license headers are inconsistent in these files: - In browser/ this file has a "#" prefix for its license headers; - mail/ and calendar/ l10n.ini files don't have a header at all; - suite/ and toolkit/ have the ";" prefix. instantbird/themes/accounts-aero.css instantbird/themes/blist-aero.css instantbird/themes/conversation-aero.css instantbird/themes/instantbird-aero.css instantbird/themes/tabbrowser-winstripe/tabbrowser-aero.css These css files start by %include'ing their non -aero version, so adding the license header in them with a /* */ comment format would duplicate the header in the resulting files. I think it's good to add a license header to these files, but it should be with a "% " prefix, so that the headers are preprocessed out. The same situation exists on comm-central. Missing headers: tools/buildbot-configs/*/mozconfig-release Subtree with files still having the tri-license headers: purple/purplexpcom/ Everything else seems fine by code inspection (I haven't tried building with the patch). Thank you very much for this patch!
Attachment #628374 - Flags: review?(florian) → review-
(In reply to Florian Quèze from comment #2) > instantbird/branding/*/mozicon*.xpm > It seems strange to put a license header in icon files, but the format > seems to accept it. However, it seems "/* XPM */" should stay at the > beginning of the file, like for "<?xml" in XML files. This also affects > files that have been touched in mozilla-central and comm-central. Wikipedia suggests that the /* XPM */ on the first line is not a requirement: http://en.wikipedia.org/wiki/XPM_%28image_format%29 Given that we've already done this on m-c and c-c, I can see value in consistency. > instantbird/installer/windows/nsis/updater_append.ini > instantbird/locales/updater_append.ini Excluded. > header here. The file installer/windows/nsis/updater_append.ini exists for > other Mozilla applications in mozilla-central and comm-central, and I think > you will want to revert the change to these files. Noted in bug 759095 and bug 760009. > instantbird/themes/smileys/theme.js > This file has the .js extension, but it actually contains JSON data, and > AFAIK that format doesn't support comments, so no license header there. I will exclude it; but you should rename it :-) > instantbird/themes/messages/*/Incoming/*.html > instantbird/themes/messages/*/NextStatus.html > instantbird/themes/messages/*/Status.html I will simply exclude instantbird/themes/messages; we can add licenses elsewhere as necessary. > tools/messagestyles/teststyles/*/*.html > tools/messagestyles/teststyles/*/*/*.html Same for tools/messagestyles/teststyles/ > Wrong comment style for the header: > > instantbird/locales/l10n.ini > The ";" prefix seems wrong here, or at least the license headers are > inconsistent in these files: > - In browser/ this file has a "#" prefix for its license headers; > - mail/ and calendar/ l10n.ini files don't have a header at all; > - suite/ and toolkit/ have the ";" prefix. Assuming these are all Windows .ini files, then the standard comment char is ";", although some software allows "#": http://en.wikipedia.org/wiki/INI_file > instantbird/themes/accounts-aero.css > instantbird/themes/blist-aero.css > instantbird/themes/conversation-aero.css > instantbird/themes/instantbird-aero.css > instantbird/themes/tabbrowser-winstripe/tabbrowser-aero.css > These css files start by %include'ing their non -aero version, so adding > the license header in them with a /* */ comment format would duplicate the > header in the resulting files. I think it's good to add a license header to > these files, but it should be with a "% " prefix, so that the headers are > preprocessed out. The same situation exists on comm-central. OK, done. > Missing headers: > tools/buildbot-configs/*/mozconfig-release Added. > Subtree with files still having the tri-license headers: > purple/purplexpcom/ Fixed. Gerv
Attached patch Patch v.2 (deleted) — Splinter Review
Try this. Gerv
Attachment #628374 - Attachment is obsolete: true
Attachment #628632 - Flags: review?(florian)
(In reply to Gervase Markham [:gerv] from comment #3) Thanks! > > instantbird/themes/smileys/theme.js > > This file has the .js extension, but it actually contains JSON data, and > > AFAIK that format doesn't support comments, so no license header there. > > I will exclude it; but you should rename it :-) That's difficult to do now, as it's also included with the same name in lots of add-ons. > > instantbird/themes/messages/*/Incoming/*.html > > instantbird/themes/messages/*/NextStatus.html > > instantbird/themes/messages/*/Status.html > > I will simply exclude instantbird/themes/messages; we can add licenses > elsewhere as necessary. I'm attaching the subset of attachment 628374 [details] [diff] [review] touching this folder that was wanted. This patch can be applied above attachment 628632 [details] [diff] [review].
Attachment #628632 - Flags: review?(florian) → review+
Sounds great. I'll let you get on with checking all these in :-) Gerv
Checked in attachment 628632 [details] [diff] [review] and attachment 628657 [details] [diff] [review] as https://hg.instantbird.org/instantbird/rev/57e8d1da63f8 And I updated by hand the tools/createheader.sh file that couldn't be handled automatically: https://hg.instantbird.org/instantbird/rev/b707ece76191 Resolving as fixed, thank you very much!
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: