Closed Bug 859648 Opened 12 years ago Closed 12 years ago

Missing copyright headers in numerous files

Categories

(Testing Graveyard :: Mozmill, defect)

Product:
Testing Graveyard
Component:
Mozmill
Type:
defect
Priority:
Not set
Severity:
minor

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: jfrench, Assigned: jfrench)

References

(
URL
)

Details

Attachments

(1 file, 2 obsolete files)

update add missing copyrights (master) rev1.2
67.11 KB, patch
whimboo
: review+
Details | Diff | Splinter Review
In the course of doing work for bug 858731, I noticed numerous files in the Mozilla/Mutt tree do not have a copyright headers in them. Both tests, and supporting files. eg. mutt/mutt/tests/js/utils/waitfor.js If you'd like me to add them where missing, feel free to assign me the bug. (intended header) /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
Good idea. Thanks!
Assignee: nobody → tojonmz
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Component: Mozmill Tests → Mozmill
Product: Mozilla QA → Testing
(intended bang and header for all .py files) #!/usr/bin/env python # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this file, # You can obtain one at http://mozilla.org/MPL/2.0/. Please let me know if you don't want the shebang added for .py files which are missing that initialization of the interpreter.
Actually, I see even the header itself, is wrongly using a capital in the middle of a sentence, and the wrap is different. I suspect if I'm going to do the work, I'll change all the files, so they are correct. So they will say: (header for js) /* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, you can obtain one at http://mozilla.org/MPL/2.0/. */ (header for py) #!/usr/bin/env python # This Source Code Form is subject to the terms of the Mozilla Public # License, v. 2.0. If a copy of the MPL was not distributed with this # file, you can obtain one at http://mozilla.org/MPL/2.0/.
The shebang we don't need given that you will not run the tests directly but through Mutt. If you want to run them directly, you can use the python interpreter. So just adding the copyright headers is fine.
Will do. I will not remove any shebangs in any py files, I will also not add any if they are absent in similar files. Only header changes.
Have completed the py files locally, about 30 files total. Need to return to bug 848649, so will return to this bug after that one is finished.
Patch "update add missing copyrights (master) rev1.0" for master. One hundred sixteen files modified. I also ran the mutt tests with my branch vs. a clean master, to ensure there were no behavioural changes between the two runs. Not that there should be just manipulating headers. Using Aurora for validation, as Default Nightly appears to have problematic with mutt tests in general the past while. Tested with Aurora 22.0a2 20130416004017. Tests pass where expected.
One newline issue with extension/content/test/test.js, that my own diffs don't show, but Bugzilla does. I will upload another patch shortly once I can correct it.
Comment on attachment 738130 [details] [diff] [review] update add missing copyrights (master) rev1.0 Based on the testing and investigation, I believe extension/content/test/test.js was never terminated with a newline to begin with, and it is now, resulting in the difference at this line https://bugzilla.mozilla.org/attachment.cgi?id=738130&action=diff#a/mozmill/mozmill/extension/content/test/test.js_sec3 So setting this patch for review. Comments are otherwise the same, as per Comment#7. I had originally wanted to inject or sed the headers into the files, but due to the inconsistent state of the files, and their header locations, this wasn't possible. So it was all hand edits. But with the repo after this bug having consistent headers, it should in theory be possible to update the MPL or the header block to different content later on, by that method.
Attachment #738130 - Flags: review?(hskupin)
Comment on attachment 738130 [details] [diff] [review] update add missing copyrights (master) rev1.0 Review of attachment 738130 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. There's just one instance of an MPL1 license block that we should replace. There are a few issues with whitespace at the end of lines, unrelated to these changes but I wonder if we should fix those in another cleanup bug. Feel free to request review from both Henrik and me when you have a new patch. ::: mozmill/mozmill/extension/resource/stdlib/securable-module.js @@ +1,1 @@ > /* ***** BEGIN LICENSE BLOCK ***** You should replace the entire MPL1 license block with the MPL2 one.
Attachment #738130 - Flags: review?(hskupin) → review-
Summary: Mutt tests - missing copyright headers in numerous files → Missing copyright headers in numerous files
Sure, sounds good. I will update that MPL1 block, and open a separate bug for the unrelated whitespace cleanup and assign myself.
Patch "update add missing copyrights (master) rev1.1" for master. One hundred sixteen files modified. Supplementary change for securable-module.js, replacing the entire license block with MPL2.0.
Attachment #738130 - Attachment is obsolete: true
Attachment #738649 - Flags: review?(hskupin)
Attachment #738649 - Flags: review?(dave.hunt)
Comment on attachment 738130 [details] [diff] [review] update add missing copyrights (master) rev1.0 Review of attachment 738130 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozmill/mozmill/extension/resource/stdlib/securable-module.js @@ +1,1 @@ > /* ***** BEGIN LICENSE BLOCK ***** We should not replace this license block! It's an import from another repository so we have to keep what it has.
Comment on attachment 738649 [details] [diff] [review] update add missing copyrights (master) rev1.1 Review of attachment 738649 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozmill/mozmill/extension/resource/stdlib/securable-module.js @@ +1,3 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this > + * file, you can obtain one at http://mozilla.org/MPL/2.0/. */ This is an external module. So we cannot change the license here. At least not until the original author gives his agreement. So I would revert to the last version.
Attachment #738649 - Flags: review?(hskupin)
Attachment #738649 - Flags: review?(dave.hunt)
Attachment #738649 - Flags: review-
Sure. So does that mean I can just mark obsolete rev1.1 of this patch, and we can use my originally uploaded rev1.0 patch? That rev1.0 patch updated just the Mozilla specific section, making it consistent with the other uses across the repo, while preserving all the rest of the 3rd party license block. And rev1.0 changed the MPL to 2.0 in the line above: (securable-module.js) * Version: MPL 2.0/GPL 2.0/LGPL 2.1 * * This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, you can obtain one at http://mozilla.org/MPL/2.0/. If you want a total revert, and this file excluded from the patch and not updated at all, let me know and I'll upload a rev1.2 patch for that.
Yes, please do not make any changes to that file and keep it as it is right now.
(In reply to Henrik Skupin (:whimboo) from comment #16) > Yes, please do not make any changes to that file and keep it as it is right > now. Before commenting I did find another instance of this file that has been updated to MPL2. I couldn't find the original file anymore, but versions in testpilot (updated to MPL2) and chromeless, but this file was renamed.
Is it the same version of the code? Where have you found it?
(In reply to Henrik Skupin (:whimboo) from comment #18) > Is it the same version of the code? Where have you found it? I did a search on mxr. I'm not at the laptop at the moment but can find it again and reference it later if needed.
It might be helpful to compare both instances. So yes, it would be good to have. Thanks Dave.
I have a patch prepared locally which leaves the file unchanged, if required.
So there are lots of code differences between that version and mozmills and we have decided for now, to exclude this file from the patch. I will upload a rev1.2 patch momentarily.
Patch "update add missing copyrights (master) rev1.2" for master. One hundred fifteen files modified. Supplementary change excluding securable-module.js for now, from the copyright updates. All other upload information the same as per Comment#7.
Attachment #738649 - Attachment is obsolete: true
Attachment #739023 - Flags: review?(hskupin)
Attachment #739023 - Flags: review?(dave.hunt)
Comment on attachment 739023 [details] [diff] [review] update add missing copyrights (master) rev1.2 Review of attachment 739023 [details] [diff] [review]: ----------------------------------------------------------------- I like it! Thanks Johannes. I will get it landed in a bit.
Attachment #739023 - Flags: review?(hskupin)
Attachment #739023 - Flags: review?(dave.hunt)
Attachment #739023 - Flags: review+
https://github.com/mozilla/mozmill/commit/5b4b9f054af51274b2b798815a64c03142e2a010
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to Henrik Skupin (:whimboo) from comment #25) > I like it! Thanks Johannes. I will get it landed in a bit. I think Henrik meant you, Jonathan. :)
(In reply to Dave Hunt (:davehunt) from comment #27) > I think Henrik meant you, Jonathan. :) Johannes works for me, it sounds cool. :)
Yes! Blame me. Give me more names! :)
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: