Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, 2011.patch, failed testing.

axyjo’s picture

Status: Needs work » Needs review
FileSize
743 bytes

Retry.

Status: Needs review » Needs work

The last submitted patch, 2011-2.patch, failed testing.

axyjo’s picture

Status: Needs work » Needs review

There's really no reason this should fail. Trying again.

Josh The Geek’s picture

#2: 2011-2.patch queued for re-testing.

Josh The Geek’s picture

Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community

I see no reason why this wouldn't work.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Since it's only a matter of time before jQuery is updated to a new version, and since John Resig still holds the copyright at jQuery, we should probably make that date 2011 as well so we don't need to revisit this in 2 weeks.

Additionally, we ship with a heck of a lot more code these days than just Drupal and jQuery... jQuery UI, jQuery Forms, BBQ, and lord knows what else. So if we're going to fix this, let's fix it right.

jhodgdon’s picture

Title: COPYRIGHT.txt still thinks it's 2010 » COPYRIGHT.txt still thinks it's 2010 and is missing some entries
Category: task » bug
Priority: Minor » Normal

Changing title, category, and priority to reflect #7

amateescu’s picture

Assigned: axyjo » Unassigned
Status: Needs work » Needs review
FileSize
1.01 KB

Here is a patch that adds the jquery plugins included in Drupal (to my knowledge, they are all located in /misc).

It's hard to find the initial release date for each plugin so i decided to leave only the year of the last release for each one.

jhodgdon’s picture

Status: Needs review » Needs work

This looks mostly good. I did notice that the jquery.js file says at the top:

* Includes Sizzle.js
 * http://sizzlejs.com/
 * Copyright 2010, The Dojo Foundation

There's also no copyright info on Farbtastic... but I found this note in the Google repository for this, which leads me to wonder what it should say:
http://code.google.com/p/farbtastic/source/browse/branches/farbtastic-1/...

Change log
r22 by matt.farina on Jan 13, 2010 

Removing header block in JavaScript containing copyright information that was added in r16. Adding this was never validated by the copyright holder. Removing until validation occurs.

But I'm also wondering if we should avoid trying to maintain a comprehensive list of the copyright dates for all included files here. It is liable to be out of date. Maybe we should just change the copyright.txt file to say:

Drupal includes works under other copyright notices and distributed according to the terms of the GNU General Public License or a compatible license. These files contain copyright and licensing notices in their headers or in their parent directories.

jhodgdon’s picture

Version: 7.x-dev » 8.x-dev

Wow, this is still a problem, even in Drupal 8. Reviving this issue; I'm sure it also needs a rerolled patch.

jhodgdon’s picture

Issue tags: +Novice, +Needs backport to D7

This is still an issue. Maybe a good Novice project?

mjonesdinero’s picture

@jhodgdon

can you elaborate here what are the exact things that will be working on her?

Thanks

jhodgdon’s picture

At a minimum the copyright dates need to be updated to 2012 in the COPYRIGHT.txt file.

yashadev’s picture

Issue tags: -Novice, -Needs backport to D7

Changed copyright dates in most of the files. COPYRIGHT.txt file already has the correct dates (at least in 8.x core).

yashadev’s picture

Changed copyright dates in most of the files. COPYRIGHT.txt file already has the correct dates (at least in 8.x core).

yashadev’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

I'm not sure what you mean in #16 by saying COPYRIGHT.txt has the correct dates, since your patch fixes the date. :)

That aside...
- I don't think we should change jquery.js dates (or any of the other JS files). That is presumably the file we got from the JQuery project, so we should only update the dates in that file if we get a new source file.
- The same applies to Twig files.
- Really, the only file we should be changing is COPYRIGHT.txt
- This patch has several unrelated changes that do not belong with this issue.

yashadev’s picture

Status: Needs work » Needs review
FileSize
879 bytes

Oh I forgot to reset the local copy, so thought it was already fixed.
Alright, my bad. The patch changes only the COPYRIGHT.txt file

Status: Needs review » Needs work
Issue tags: -Novice, -Needs backport to D7

The last submitted patch, copyright-dates-1012018-19.patch, failed testing.

yashadev’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +Needs backport to D7

#19: copyright-dates-1012018-19.patch queued for re-testing.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

That looks good, thanks! I'll get it committed, and then we can backport to D7.

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

Oh sorry. I just read through the comments on this issue and noticed #7, #9, and #10 -- in other words, there are more entries needed in COPYRIGHT.txt . A patch with some of them was included in #9, and some other suggestions are in #10.

yashadev’s picture

Status: Needs work » Needs review
FileSize
1015 bytes

Here you gooo, let me know if that's not all

jhodgdon’s picture

Status: Needs review » Needs work

That looks like a good list... A few problems:

a) The patch is now missing the year change needed at the top of COPYRIGHT.txt.

b) I had thought the idea was that for all of the third-party code we are using, we would update the copyrights to say (year) - 2012, like this patch does for JQuery... But I think your approach is better. So let's just take the actual copyright information from the current files for everything...
- jQuery form - looks like the date at the top is 2011, not 2010.
- JQuery itself - file date says 2011 only.

c) I think it would be nice to put the JS entries together, and the non-JS entries together (non-JS = Symfony and Twig; I think everything else is JS).

d) I checked the files in the core/misc directory, and I think html5.js needs to be added to the list.

e) I know of one more non-JS file that we need to include: core/lib/Drupal/Component/Archiver/ArchiveTar.php

Thanks!

yashadev’s picture

Assigned: Unassigned » yashadev
Status: Needs work » Needs review
FileSize
1.47 KB

Thanks, I also changed all of the copyright dates of the libraries in format (year) instead of (year) - (year) so all of them follow some sort of format.

jhodgdon’s picture

Status: Needs review » Needs work

Looks pretty good!

There are a couple of spaces at the ends of lines, which need to be removed (you should be able to set up your programming text editor to either display or remove them).

Also, I think that the copyright years has to follow exactly what is in the other files. So if the other file says "Copyright 2005-2012", we should do that too. It looks like Symfony should be:
Copyright (c) 2004-2012 Fabien Potencier
And Twig is
Copyright (c) 2009 by the Twig Team
and ArchiveTar is
Copyright (c) 1997-2008,

I didn't check over the JS entries, can you do that please?

yashadev’s picture

Status: Needs work » Needs review
FileSize
1.22 KB

Should be all.
Also, I found another one, YUI - Copyright (c) 2010 Yahoo! Inc. in \core\vendor\Symfony\Component\HttpKernel\Debug\ExceptionHandler.php

I know it's in the symfony folder.. So wasn't sure if it should be included. Correct me if I'm wrong

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks good! Thanks for carrying this through all the iterations. I'll get it committed as soon as possible.

tstoeckler’s picture

Status: Reviewed & tested by the community » Needs work

This is missing the Drupal part at the top of the file. Don't know if that can be fixed pre-commit, but marking "needs work" anyway.

yashadev’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

Aghh missed that again

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Yay! :-)

jhodgdon’s picture

Thanks tstoeckler, I missed that in my last review too. :)

jhodgdon’s picture

Status: Reviewed & tested by the community » Needs work

Uh oh. I applied this patch and noticed that a couple of the lines are now longer than 80 characters. They need to wrap at 80 characters.

yashadev’s picture

Status: Needs work » Needs review
FileSize
1.52 KB

So should they just be moved to a new line?
Heree

jhodgdon’s picture

Yes, that's the right idea. :) This is probably fine, but I haven't given it a full review so I'm not yet setting it to RTBC. Thanks!

jhodgdon’s picture

Status: Needs review » Needs work

I carefully reviewed each line of this patch. It's just about all correct, but I found:

a) Sizzle -- core/misc/jquery.js says Sizzle is copyright 2011. The patch says 2010.

b) jQuery UI -- I grepped for "Copyright" in directory core/misc/ui/ui, and all the files seem to be copyright 2012, but this patch says 2011.

c) I found JQuery Cookie in directory core/misc/ui/external. It looks like there are several other files in that directory that need to be included here?

d) In the core/vendor/Symfony directory, I did a grep for Copyright and it appears that a couple of the Symfony components or files have different copyrights: HttpFoundation/Request.php, HttpKernel/Debug/ExceptionHandler.php

mjonesdinero’s picture

Assigned: yashadev » mjonesdinero

assigning to my self as one week has already passed..

mjonesdinero’s picture

here is updated patch i have done..

mjonesdinero’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Needs work

This patch is missing some of the patch in #35 (the top few lines). I haven't looked at the rest yet.

mjonesdinero’s picture

Status: Needs work » Needs review
FileSize
1.99 KB

another updated patch..

mjonesdinero’s picture

sorry for the wrong comment # on the patch attached

jhodgdon’s picture

Status: Needs review » Needs work

Thanks -- this patch is much better! The copyright file still needs some work though:

a) Remove blank line at the top of the file. Also I don't think we need two blank lines between the "Drupal includes works..." paragraph and the beginning of the list.

b) A couple of lines need to be wrapped at 80 characters.

c) The "JQuery Metadata" line is not formatted the same as the others (no space and hyphen before Copyright).

d) Things are not really in a logical order... How about if we make a section called "JavaScript" and another called "PHP", and put everything in alphabetical order within each section?

e) Globalize is a JQuery library, so maybe it should be called "JQuery Globalize" in the list?

f) Globalize does not have a date on its copyright. Granted, I don't see one on their github web site either, but it looks like at least some of the files there were updated this year, so let's just put in 2012.

g) JQuery Bigframe should be JQuery bgiframe.

h) The date is wrong on JQuery Cookie (2010).

i) On the Symfony components... I think we should format it like this (rather than having separate entries from YUI and Zend):
Symfony2 - Copyright (c) 2004 - 2012 Fabien Potencier, with additional code from:
- YUI - Copyright (c) 2010, Yahoo! Inc.
- Zend Framework (1.10dev - 2010-01-24) - Copyright (c) 2005-2010 Zend Technologies USA Inc. (http://www.zend.com)

mjonesdinero’s picture

Status: Needs work » Needs review
FileSize
2.02 KB

@jhodgdon

here is it now..updated patch

jhodgdon’s picture

Status: Needs review » Needs work

Thanks, looking better! I think in one more iteration we will hopefully have this done :)

a) Anywhere with two blank lines in a row, please remove one of them. Plus, we don't need an extra blank line at the end of the file.

b) The "JQuery metadata" lines:

  jQuery Metadata - Copyright (c) 2006 John Resig, Yehuda Katz, J?örn Zaefferer
    , Paul McLanahan

- What is that question mark in Zaefferer's first name?
- The comma should be after Zaefferer (not separated by a space and definitely not at the beginning of a line)

c) Farbtastic is a JQuery plugin.

d) HTML Shiv is JavaScript.

mjonesdinero’s picture

Status: Needs work » Needs review
FileSize
2.04 KB

here it is now..

thanks for the feedbacks

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Needs review » Patch (to be ported)

In an issue where you are adding new patches to fix problems identified in previous patches, it is very helpful to include an "interdiff": http://drupal.org/node/1488712 :)

Anyway, I think this is good to go (I fixed the newline that was missing at the end of the file -- we didn't want an extra blank line, but we do want a carriage return at the end). So, committed to 8.x

Now we need to do something similar for 7.x. The overall Drupal copyright needs to be adjusted to 2012, and then we need to figure out what 3rd-party code we have, and the dates. I think the 8.x file would be a good starting point, but for instance we don't have Symfony or Twig in 7.x. We do have ArchiveTar and I think most of the JS libraries, but the JS libraries may not have the same copyright dates.

mjonesdinero’s picture

setting this as unassigned, i cant work as of this time with the porting

Albert Volkman’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.06 KB

D7 backport.

jhodgdon’s picture

Status: Needs review » Needs work

That's a good start, thanks!

A few things to fix:
- We need to include ArchiveTar (it's in modules/system/system.tar.inc).
- In the misc directory, it looks like jQuery Cookie needs to be included.
- Some dates are different for D7: JQuery UI - 2010, JQuery - 2010, jquery form - 2010, sizzle - 2010.

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
913 bytes
1.17 KB

Here ya go!

jhodgdon’s picture

Status: Needs review » Needs work

I think the JQuery Cookie date should be 2006 for D7? But other than that, looks good, thanks!

Albert Volkman’s picture

Status: Needs work » Needs review
FileSize
1.17 KB

Done!

jhodgdon’s picture

Version: 7.x-dev » 6.x-dev
Status: Needs review » Patch (to be ported)

Looks good, thanks! Committed to 7.x.

I guess we should at least take a look at 6.x and see if it needs updates too?

logowallpaper’s picture

Tested, is working.

Albert Volkman’s picture

Status: Patch (to be ported) » Needs review
FileSize
796 bytes

D6 backport.

mjonesdinero’s picture

Assigned: mjonesdinero » Unassigned
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks! Those dates appear to be correct, and I think the D6 patch is complete (includes all the third-party libraries in the "misc" directory anyway).

jhodgdon’s picture

Status: Reviewed & tested by the community » Fixed

Wow. When this issue started, it was 2011. By the end, it was 2012, and we're about to need a new issue for 2013.

In any case, I've committed this patch for 6.x, so let's close this one up and someone can file a new one for 2013 in 2 months. :)

Status: Fixed » Closed (fixed)
Issue tags: -Novice, -Needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.

cilefen’s picture