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

Required files check #128

Open
ernilambar opened this issue Mar 6, 2019 · 7 comments
Open

Required files check #128

ernilambar opened this issue Mar 6, 2019 · 7 comments
Labels
Status: Future Release This issue will be fixed in the future release of the plugin, or an enhancement will be added in the Status: Possible Bug This issue is a possible bug, but might not be (could be a misunderstanding or a false positive). Type: Enhancement This is an enhancement of the existing features or a new feature to the plugin.

Comments

@ernilambar
Copy link
Member

Related to #115

  • Readme could be readme.txt or README.txt or readme.md. (Case insensitive and valid extensions are txt and md)
  • Screenshot could be screenshot.jpg or screenshot.png
@dingo-d dingo-d added Type: Enhancement This is an enhancement of the existing features or a new feature to the plugin. Status: Future Release This issue will be fixed in the future release of the plugin, or an enhancement will be added in the Status: Possible Bug This issue is a possible bug, but might not be (could be a misunderstanding or a false positive). labels Mar 6, 2019
@timelsass
Copy link
Member

If there's a standard documented, then it should be followed is my thought.

For png/jpg - this has been acceptable in the past, but the requirement for this was being based off of https://developer.wordpress.org/themes/release/required-theme-files/ verbatim.

Given that there's actually a screenshot_url in the .org themes api - why even limit screenshots to a particular format, and allow an author to use a format of their liking. Theme sniffer could allow multiple, and warn for preferred format. There should be a standard to the format though as everything else does. I'm not sure how we would go about updating that page, but if the standard were to change from screenshot.png to allow for either or screenshot.jpg then I think we could change it. I also think it could stand to be more permissive - perhaps webp as the recommended format (requiring a fallback), which would not only help keep distribution weight a tiny bit smaller, but help reduce .org resource usage over time. The screenshot is pretty much just for being displayed on .org. We could always have a fix for this sniff which would convert invalid types into the required type so the proper format is only a click away.

For the readme, this was used for reference: https://make.wordpress.org/themes/handbook/review/required/#readme-txt-file

I have an issue with calling a README.md file the same thing as a readme.txt, it's misleading. Extensions are used to determine content by lots of applications that people working with this tool are likely to be using - such as ides, code editors etc. You add unnecessary overhead by trying to parse a .md when it's not even an actual .md, and conflicts. Sure now I have the option to say this a readme.txt file, but it becomes a nuisance opening other projects causing some lag for some of the processes when there was no reason to because the editor recognized it as a particular file which has expected behaviors. Github for instance will attempt to read the .md file and display a render of the contents. This doesn't help out anything when the standards aren't even a standard format that anyone recognizes or is really able to be parsed. Which is the first big problem I believe this format has.

I know the "parser" can attempt to figure out some markdown, but there's so much overhead in the whole process. There's checks for single one-off use cases, which help cover converting some usecases, but not others that would be logically used.

Take the following example of using setext style headers:

Testing Plugin
===========

There's code in the .org script to check for this specific scenario, so it gets formatted the same as the standard:

=== Testing Plugin ===

An author who is using setext style they used it for a reason, so it's safe to assume they are going to write other content in their readme using it, and logically that would be for sections:

FAQ
------

This doesn't validate as the section and you'll have a notice for missing FAQ section. So the only acceptable place for setext style headers is the plugin name, which is weird.

There's a lot of inconsistencies overall, and I've looked a lot of readmes being approved, and can see the issue beginning to crop up. If we want to parse and validate resources - we have to enforce a standard format. People are doing all sorts of weird and/or creative things - some aren't even markdown, some are to make things prettier to them, and most don't even have all of the same information. Regardless we'll end up with an impossible attempt and dirty regex match that's always going to be error prone that no one will work with because it sucks.

Why aren't there any editor plugins that format your WordPress readmes?
Why aren't there any build tools to lint WordPress readmes?

There's some that "lint", and by that I mean it just checks for some of the required fields via regex, but it won't tell you if you've formatted outside of a standard spec, which is a major problem if we want to glean information from it and provide warnings, errors, and fixes in a linting fashion.

If you really sit down and think about it - if the readme file was a structured format, this problem wouldn't exist. It's because we allow variations of random things that are inconsistent, such as invalid file types

Taking the example code above show clear evidence:

  • this is how github displays "WordPress Markdown"

=== Testing Plugin ===
Requires at least field: 5.0

== Changelog ==
=1.0=

  • changed something

this is how github displays the actual markdown if used in a file with the extension .md, or in allowed areas, such as comments:

Testing Plugin

Requires at least field: 5.0

Changelog

1.0

  • changed something

The lax readme format does make things like building an actual parser or linter a bit ridiculous, especially given that .org's "parser" is more so a "formatter and renderer all in one" not a parsersince we can't really gather any token or AST. This is what the other sniffs are doing and are able to do, so a sniff on a readme should be the same. Success for other package managers has been found by using universally recognized way of describing projects in a repo, such as inclusion of that information in a .json or .yaml. WordPress' known type has been readme.txt, so if we're considering making the standard .md and allowing variations of naming conventions, then I would urge moving to one of those formats instead :P

I do have some changes that I think would make sense as I started looking into the requirements, initial validation checks for resources' licenses, and the readme.

If you have any interest in this - or anyone else for that matter, this is my initial WIP.

Some changes that aren't documented in that initial draft include:

  • some changes to the formatting specified for the resources section to better align with changelog - ie header includes resource name and version, opposed to only a list with a comma followed by a single space delimiter.
  • After reviewing several readme.txt's I noticed some that use invalid license identifiers which aren't of a known or recognized format. So an increase to the license vocabulary.

@carolinan
Copy link

Looks like the directory requires the sceeenshot to be a png:

https://meta.svn.wordpress.org/sites/trunk/wordpress.org/public_html/wp-content/plugins/theme-directory/class-themes-api.php

@carolinan
Copy link

I am for requiring readme.txt to start with, regardless of the format. We can keep working on the format.

@joyously
Copy link

Looks like the directory requires the sceeenshot to be a png

It's not required. My theme uses jpg, and I know a bunch of themes do also, especially old ones.

@dingo-d
Copy link
Member

dingo-d commented Mar 30, 2019

Yeah, just looked at my theme and it's jpg 🙂

timelsass referenced this issue in timelsass/theme-sniffer Mar 31, 2019
@joyously
Copy link

joyously commented Jun 4, 2019

We should require readme.txt and ignore readme.md.

@dingo-d
Copy link
Member

dingo-d commented Jun 4, 2019

I agree, if we want to encourage people to write uniform readmes, we can notify if the readme.txt is missing or not. I wouldn't forbid the readme.md but it shouldn't be checked imo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Future Release This issue will be fixed in the future release of the plugin, or an enhancement will be added in the Status: Possible Bug This issue is a possible bug, but might not be (could be a misunderstanding or a false positive). Type: Enhancement This is an enhancement of the existing features or a new feature to the plugin.
Projects
None yet
Development

No branches or pull requests

5 participants