plugin-check icon indicating copy to clipboard operation
plugin-check copied to clipboard

Expand readme check

Open swissspidy opened this issue 2 years ago • 7 comments

Is your enhancement related to a problem? Please describe.

Follow-up to #178

The current check is rather simple, but there are many more things that can be checked. I actually wrote a CLI command for that last year, which should be pretty easy to port over: https://github.com/swissspidy/validate-readme-command

Designs

No response

Describe alternatives you've considered

No response

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

swissspidy avatar Jun 21 '23 11:06 swissspidy

While working on https://github.com/WordPress/plugin-check/pull/392 I found few issues in the library. Parser is working as expected for the display of the file content. But in our case we need raw value which the class is not providing.

Example: Parser given trimmed version (150 characters) of short description. So we cannot check if actual short description is longer than 150 characters.

Another case, only 5 tags (sliced version) are returned. So it is not possible to warn if readme has more than 5 tags.

This limits our ability to expand more detailed readme check.

ernilambar avatar Jan 16 '24 06:01 ernilambar

Oh that is a good point. I see a few possible ways to address this:

  1. Submit changes upstream to the meta parser to allow retrieving raw values
  2. Use composer patches to add this only in this repo
  3. Fork the code

swissspidy avatar Jan 16 '24 08:01 swissspidy

@swissspidy I have created issue here in the repo https://github.com/afragen/wordpress-plugin-readme-parser/issues/2 Any other data to be returned which we can use in our plugin?

ernilambar avatar Jan 17 '24 10:01 ernilambar

Note that https://github.com/afragen/wordpress-plugin-readme-parser is just a copy of https://meta.trac.wordpress.org/browser/sites/trunk/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php

So if we want to expand the parser (option 1), the ticket/PR would need to be created on Meta Trac.

But before we do any of that we first need to decide which option to actually take

swissspidy avatar Jan 17 '24 11:01 swissspidy

Another case, only 5 tags (sliced version) are returned. So it is not possible to warn if readme has more than 5 tags.

Some things trigger a "warning" in the readme parser which can be used to detect such things, for example: https://github.com/WordPress/wordpress.org/blob/1cb4236b813ba7c296d7a4f986352996879432f9/wordpress.org/public_html/wp-content/plugins/plugin-directory/readme/class-parser.php#L637

Unfortunately too-many-tags is not one of them, nor is short-description-too-long.

I don't think it's worth adding a way to get the original content from the readme to that parser, but adding additional warnings there for cases you'd like to catch seems worthwhile to me.

For example; here's a quick PR I mocked up; https://github.com/dd32/wordpress.org/compare/trunk...plugins/plugin-check-198 If you create a meta.trac ticket I'll submit it as a PR example that you can build off or request additional flags.

dd32 avatar Jan 18 '24 03:01 dd32

Meta ticket: https://meta.trac.wordpress.org/ticket/7412

ernilambar avatar Jan 19 '24 04:01 ernilambar

Following readme related issues and PRs are open and under review.

  • https://github.com/WordPress/plugin-check/issues/402
  • https://github.com/WordPress/plugin-check/issues/405
  • https://github.com/WordPress/plugin-check/pull/392

After these issues are resolved, we could mark this issue as completed.

ernilambar avatar Feb 09 '24 07:02 ernilambar