PHPDoc @license Tag Review

[hakre]

Syntax as ​specified. Helps to normalize as well.

Related: #14727

Second patch looks good except for a few issues:

• Added subpackage to PemFTP files, and I suppose to fix the Pure class and the Socket class from not showing up in nightlies. The fix for this is simple without adding subpackage and should be fixed in a new ticket.
• The patch is dirty in some places making changes not related to the @license phpdoc tag. Some examples include the UTF8 translation to ANSI function.
• Patch doesn't appear to be accepted format for which Trac can read. I'm not sure why this is, but it would be helpful.

Replying to jacobsantos:

Second patch looks good except for a few issues:

• Added subpackage to PemFTP files, and I suppose to fix the Pure class and the Socket class from not showing up in nightlies. The fix for this is simple without adding subpackage and should be fixed in a new ticket.

I don't understand what that means, but if it's easy to fix, good :)

• The patch is dirty in some places making changes not related to the @license phpdoc tag. Some examples include the UTF8 translation to ANSI function.

Do you mean character encoding in /wp-includes/class-snoopy.php / _striptext() line 720 and something?

• Patch doesn't appear to be accepted format for which Trac can read. I'm not sure why this is, but it would be helpful.

Patch does exploit tracs display, ​I reported it upstream.

Thanks for the review, please let me know for the other two points. with the encoding of the snoopy class it looks like that those values need to be properly encoded in the PHP code instead of just putting them in. I think the changes happens on my side becaue I checkout the whole project as UTF-8.

Do you have the binary values that are used in there? Maybe that is exploiting the trac display as well? Should we create another ticket to just fix the encoding issue in the snoopy class so we get it ASCII-7BIT save?

Replying to hakre:

Replying to jacobsantos:

Second patch looks good except for a few issues:

• Added subpackage to PemFTP files, and I suppose to fix the Pure class and the Socket class from not showing up in nightlies. The fix for this is simple without adding subpackage and should be fixed in a new ticket.

I don't understand what that means, but if it's easy to fix, good :)

Well, I was looking at ​WordPress phpdoc web site and the Pure and the Socket classes weren't showing up. The files were, so I'm guessing that the classes do not have class level phpdoc which is why they aren't showing up. This should be a in a new ticket to fix that.

I actually have no plans to fix it though, I was simply pointing out that it doesn't belong in this ticket and that the fix is relatively simple to do, if not time consuming, but I guess a simple line or two putting the class in the right place would be easy enough.

• The patch is dirty in some places making changes not related to the @license phpdoc tag. Some examples include the UTF8 translation to ANSI function.

Do you mean character encoding in /wp-includes/class-snoopy.php / _striptext() line 720 and something?

About the character encoding. I'm not sure. I realize that the encoding of the file tends to mess special characters up, but they shouldn't be included in the patch. As it would mess up the file when the patch is applied.

If you are using tortoisesvn, then you could just view changes and then revert the changed characters. I'm not sure what will happen if the encoding hasn't changed. You might have to fix that on your end with a new check out or simply copying the file over checking out the file(s) with the correct encoding.

2nd iteration, tagging untagged files. (fixed encoding)

I created a new version of the patch with fixed encoding.

---

Snoopy / Encoding (and I found another file): #14735

Replying to jacobsantos:

Well, I was looking at ​WordPress phpdoc web site and the Pure and the Socket classes weren't showing up. The files were, so I'm guessing that the classes do not have class level phpdoc which is why they aren't showing up. This should be a in a new ticket to fix that.

I can not say why that is, normally I would check PHPDOCs log for issues it's running over.

---

Everything else should be addressed now in the patch.

Related: #14944

Related: #14783

Refreshed against trunk, patch was stale. Removed the @subpackage tags from the PEMMFTP package files as it is not in the scope of this ticket.

Streamlined the patch now. Run over an encoding issue in wp-includes/Text/Diff/Engine/string.php as it contains some ANSI chars (Ö) that got broken with UTF8. Should be okay to commit as I left that file in ANSI encoding.

Related: #15585

Generally, I'd prefer linking to ​http://www.opensource.org/licenses/ for all licenses, as the FSF landing pages can be a little political and promotional of their newest versions.

We should not be modifying external libraries unnecessarily.