TYPO3GmbH / site-intercept

GNU General Public License v2.0
2 stars 6 forks source link

What are the reasons for the very strict composer.json constraints for documentation to be rendered on docs.typo3.org #57

Closed helhum closed 2 years ago

helhum commented 2 years ago

Currently for documentation of a package to be rendered on docs.typo3.org this package not only needs to be registered (and be approved) manually, but must also pass some automated checks regarding the composer.json

This does not work for packages like helhum/typo3-console (very complex workaround is in place to circumvent these constraints) or typo3/coding-standards (not yet on docs.typo3.org).

If there is a manual approval workflow anyway, why are there these strict automated tests as well? What is the purpose to have them?

Lefaux commented 2 years ago

From the top of my head (from very long ago): The tests are executed when the webhook sends new payload. So every time any change to composer.json happens, the tests are run. Technically someone could create a new commit that removes composer.json from the repo entirely and IIRC we need composer.json to render the docs.

The initial reason for the manual approval was (also) a legal one, because legally speaking the T3A is liable for content on docs.typo3.org (even though it's user generated). The assumption is that if people provide proper docs once the likelyhood of them using a later commit to render harmful content was considered low.

Please don't take my reply as the answer to everything, I'm sure others will add more reasons, these are just the ones I remember.

helhum commented 2 years ago

we need composer.json to render the docs.

May check for what really is needed for the rendering? type for example likely isn't. That is why I am asking. What are the minimum requirements for rendering and publishing the documentation? And then maybe: Can we work on those to ease publishing of documentation for packages that aren't "Extensions"?

The initial reason for the manual approval was (also) a legal one, because legally speaking the T3A is liable for content on docs.typo3.org (even though it's user generated).

A agree on the manual approval. This makes absolutely sense to avoid spam and/or scam. Imho it mostly needs to be checked if the person owning the submitted repo is credible (e.g. registered on typo3.org). If that is the case, then approval should be given.

The assumption is that if people provide proper docs once the likelyhood of them using a later commit to render harmful content was considered low

Agreed as well.

susannemoog commented 2 years ago

At the moment, the manual approval process is more about validating the person/organisation sending the hook/rendering request than about the validity of the rendered content, we are usually not looking at the composer.json manually. One of the arguments of having the typo3 core dependency back then was that we should only render TYPO3 documentation and not "any package on the planet". However, from today's point of view, I would probably remove that check, as the likelihood of someone using the TYPO3 documentation server to render an absolutely unrelated package is quite small. As the distinction between extension and not is relevant at the moment for where the documentation ends up (URL) and for things like the extension search/list that is generated, we would need to check first if that has any further impact - because we would not want surf or tailor to be listed as extensions suddenly. But these are all solvable problems, and if it allows us to remove some of the hoops we are currently jumping through to render surf, tailor etc. it's imho worth it.

susannemoog commented 2 years ago

Please test if the latest version of intercept is enough for you - we removed the requirement for the typo3/core dependency. Feel free to reopen this issue if there is still something you need from us :)

helhum commented 2 years ago

we removed the requirement for the typo3/core dependency

Did you remove the requirement for "type": "typo3-cms-extension" as well. Didn't see any change related to that.