Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error no_license not triggered with missing readme file #737

Open
ernilambar opened this issue Oct 22, 2024 · 4 comments · May be fixed by #746
Open

Error no_license not triggered with missing readme file #737

ernilambar opened this issue Oct 22, 2024 · 4 comments · May be fixed by #746
Labels
[Team] Plugin Review Issues owned by Plugin Review Team [Type] Bug An existing feature is broken

Comments

@ernilambar
Copy link
Member

When there is no readme file, "no_license" is not triggered even when there is no "License" header in plugin main file.

This is happening because license check is under Plugin_Readme_Check. May be we should separate license check for plugin header and readme file.

Check Plugin_Header_Fields_Check should be better place for License plugin header check.

Also same error code no_license is used for missing license in readme and plugin header which is confusing.
@ernilambar ernilambar added [Team] Plugin Review Issues owned by Plugin Review Team [Type] Bug An existing feature is broken labels Oct 22, 2024
@davidperezgar
Copy link
Member

But, it getting the error for readme. I think that would be enough. After the user makes the readme, it will take the license problem. I think there is no problem with that. But do you think then is important to warn the license issue as well in one step then?

@ernilambar
Copy link
Member Author

Although not accepted by the directory, PCP still supports single file plugin. So we should not assume that readme will be always be there.

@ernilambar
Copy link
Member Author

@davidperezgar

For validating license in readme, we are using regex like this:

'/^([a-z0-9\-\+\.]+)(\sor\s([a-z0-9\-\+\.]+))*$/i'

But for license validation in plugin header, we are using like this:

'/GPL|GNU|MIT|FreeBSD|New BSD|BSD-3-Clause|BSD 3 Clause|OpenLDAP|Expat|Apache/im'

Any particular reason why these two are checked in different way?

@davidperezgar
Copy link
Member

No, there is no reason for that. And you can update with MPL 2.0 support #719 .

@ernilambar ernilambar linked a pull request Oct 25, 2024 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Team] Plugin Review Issues owned by Plugin Review Team [Type] Bug An existing feature is broken
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

License check improvements WordPress/plugin-check
2 participants
@ernilambar @davidperezgar