Closed Bug 759808 Opened 8 years ago Closed 8 years ago

MPL 2 upgrade: Instantbird

Categories

(mozilla.org :: Licensing, task)

task
Not set

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) — 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.2Splinter 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: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.