Closed
Bug 759808
Opened 13 years ago
Closed 13 years ago
MPL 2 upgrade: Instantbird
Categories
(mozilla.org :: Licensing, task)
mozilla.org
Licensing
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: gerv, Assigned: gerv)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
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
Updated•13 years ago
|
Summary: MPL 2 upgrade: InstantBird → MPL 2 upgrade: Instantbird
Assignee | ||
Comment 1•13 years ago
|
||
How does this look?
Gerv
Attachment #628374 -
Flags: review?(florian)
Updated•13 years ago
|
Blocks: Instantbird
Comment 2•13 years ago
|
||
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-
Assignee | ||
Comment 3•13 years ago
|
||
(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
Assignee | ||
Comment 4•13 years ago
|
||
Try this.
Gerv
Attachment #628374 -
Attachment is obsolete: true
Attachment #628632 -
Flags: review?(florian)
Comment 5•13 years ago
|
||
(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].
Updated•13 years ago
|
Attachment #628632 -
Flags: review?(florian) → review+
Assignee | ||
Comment 6•13 years ago
|
||
Sounds great. I'll let you get on with checking all these in :-)
Gerv
Comment 7•13 years ago
|
||
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.
Description
•