Skip to content
This repository has been archived by the owner on Sep 27, 2023. It is now read-only.

Feature/validate readme #139

Merged
merged 9 commits into from
Mar 30, 2019
Merged

Conversation

timelsass
Copy link
Member

This does a basic readme.txt sweep. Once info is parsed the following is checked:

License

SPDX/FSF ID Matches

Checks if supplied text for license field is a FSF or SPDX ID. SPDX is preferred, and warning for FSF usage are given with the correct SPDX ID provided in those instances if it could be determined.

SPDX/FSF name matches.

Will check if a valid SPDX or FSF name match is found. This is not case insensitive, just as the one above is not either. They all require users to use exact license matches. This ensures they are submitting properly licensed themes in the repository where the license identification is universal and recognized.

So using GPL-2.0-or-later as an example, these are acceptable in the License field:
GNU General Public License v2.0 or later
GNU General Public License (GPL) version 2

This would not be:
GPLv2

Non-standard Identifiers

Some of the FSF lingo is more non-standard, and doesn't correlate 1 to 1 to a SPDX counterpart. In the above section, the example shows GPLv2 is not a standardized license identifier, and could mean GPL-2.0-or-later, GPL-2.0-only, or their deprecated counterparts GPL-2.0+, and GPL-2.0. In those situations, it should warn and not error. They have a few license terms which sometimes refer to multiple, but it's not very common (aside from the example of GPL 😆 ).

Deprecated License Identifiers

Deprecated License identifiers are accounted for, so if a user does try something like GPL-2.0+, then it will error letting them know it's deprecated.

I have not added anything in to notify them of the appropriate replacement identifier in those cases though.

GPL compatibility

Standard checks on the license field, once a valid license identifier is found - then a gpl compatibility check is done - and throws errors if the supplied license is not GPL-2.0-or-later compatible.

License URI

License URIs are checked against valid license's supplied. Errors are thrown, and proper long life URI's are provided, so the author can easily change to an approved license URI. All licenses have multiple URIs - some as little as 2, others more. Take for example:

Imlib2, which is the SPDX identifier for Imlib2 License - it has these URIs available:
http://trac.enlightenment.org/e/browser/trunk/imlib2/COPYING
https://git.enlightenment.org/legacy/imlib2.git/tree/COPYING
http://spdx.org/licenses/Imlib2.json
https://www.gnu.org/licenses/license-list.html#imlib
http://directory.fsf.org/wiki/License:Imlib2

Listing any of those for the License URI will allow theme-sniffer to pass license validation for a theme claiming Imlib2 license.

Contributors

Will check against .org user profile for each contributor listed. Will error when the user doesn't exist as a required field if they choose to add contributors && warn for call failures. It might be worth add a short lived cache per each for people who run multiple times, but I didn't implement anything for that.

bin/build.js Outdated Show resolved Hide resolved
bin/build.js Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
*/

// Ignoring this from sniffs for now since the majority was pulled from .org to retain compatibility.
// phpcs:ignoreFile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fan of this 😕

Since this is an OS project, I'd like for it to present the best practices 🙂There don't seem to be many issues in this file that can be fixed

Copy link
Member

@dingo-d dingo-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments while the rain is pouring in Sidney 😄

Like the general outlook of the PR, a lot is done in it, great work 👍

@timelsass
Copy link
Member Author

For the remaining items you cited:
husky script: 37a4f8c
This just changes husky to just run the npm script instead of adding it to husky where we can't access it, so the best of both worlds 😄 .

Set up license data repo, pull in as dep: c3fc0fa
Created the initial repository/readme/etc and updated theme sniffer's build process. This will let theme sniffer use/update versions as needed for it's releases.

@dingo-d
Copy link
Member

dingo-d commented Mar 30, 2019

Ok, this looks good for now, we'll refactor things along the way (the sniff callback is getting chunky atm 😄)

@dingo-d dingo-d merged commit 1e35590 into WPTT:development Mar 30, 2019
@timelsass
Copy link
Member Author

Ok, this looks good for now, we'll refactor things along the way (the sniff callback is getting chunky atm )

Yeah 😃 I think #160 starts on progress towards slimming it out, and that PR has the changes from @pattonwebz included from #129. I pulled them in first before reorganizing so we didn't become too diverged

@dingo-d
Copy link
Member

dingo-d commented May 20, 2019

Once we sort the WPThemeSniffer, I plan on preparing Theme Sniffer for an update. I'd like to add php-di as a dependency injection container (https://github.com/PHP-DI/PHP-DI), add more interfaces, and definitely split the sniff mega class to smaller classes to minimise breaking SRP and some other SOLID principles 😄

We could set up some meeting about how it should be best to approach this 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants