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.

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

Remove development files from releases #276

Open
wants to merge 1 commit into
base: v1.x
Choose a base branch
from

Conversation

phansys
Copy link
Contributor

@phansys phansys commented May 21, 2021

No description provided.

@Art4
Copy link
Collaborator

Art4 commented May 22, 2021

This will eventually cause some problems with the building of Gentoo packages. See #201
Ping @orlitzky

@orlitzky
Copy link
Contributor

Nothing has changed here. The tests and docs belong in the release on principle: end-users need them. The whole purpose of the tests is to ensure that the thing runs on the user's system, right? And who else would the documentation be for?

The Gentoo package will still work if you delete them from the release, since we install/run whatever is there, but the overall quality of the package will be reduced. I realize that composer just blindly copy/pastes all of this crap into your public webroot, but you shouldn't penalize the rest of us to make its retarded way of doing things slightly less retarded =P

@phansys
Copy link
Contributor Author

phansys commented May 22, 2021

Nothing has changed here. The tests and docs belong in the release on principle: end-users need them. The whole purpose of the tests is to ensure that the thing runs on the user's system, right? And who else would the documentation be for?

The reasoning behind considering these files as development ones is that they are not required in order to use the package. They are only required for development purposes and will be never used in runtime.
IMO, there is no need for end users to run the test suite, as from its perspective, they are consuming a dependency which is already tested by its own CI pipeline, which ensures the expected quality in every release.

The Gentoo package will still work if you delete them from the release, since we install/run whatever is there, but the overall quality of the package will be reduced. I realize that composer just blindly copy/pastes all of this crap into your public webroot, but you shouldn't penalize the rest of us to make its retarded way of doing things slightly less retarded =P

What points are you considering about the overall quality?
How do you think it will be reduced if these files are removed?
Which scenarios are you thinking about when you say someone will be penalized with this change?

@orlitzky
Copy link
Contributor

The reasoning behind considering these files as development ones is that they are not required in order to use the package. They are only required for development purposes and will be never used in runtime.
IMO, there is no need for end users to run the test suite, as from its perspective, they are consuming a dependency which is already tested by its own CI pipeline, which ensures the expected quality in every release.

But that's not true! The CI tests only two specific configurations. There's a huge (combinatorial, impossible to try them all) number of valid configurations. All matching versions of all dependencies, as well as of PHP itself, with yes/no for every optional PHP extension. Then there is the host machine to consider... it's architecture, how much RAM is available to PHP, what's in php.conf, etc.

Allowing users to run the tests before they install the package ensures that they don't have an untested and broken configuration (this is all automatic in Gentoo, and the package install will fail if the tests fail).

What points are you considering about the overall quality?
How do you think it will be reduced if these files are removed?
Which scenarios are you thinking about when you say someone will be penalized with this change?

I'm talking only about the quality of the release tarball, which composer conflates with the thing to be copied into your webroot. A priori, having the documentation and tests in the release tarball can only add to that quality; not harm it. The documentation is useful to some users, and does no harm to others. The tests are useful to everyone IMO, and do no harm to others.

The sane way to install the (any) library is to put one copy, managed by the system, under PHP's include_dir. The tests should be run and then deleted; i.e. not installed. The documentation goes wherever your OS puts documentation. This is what we do, and deleting the documentation and tests only makes things worse: users don't get the docs and can't run the tests, and we never had any of composer's problems in the first place, so nothing is gained in exchange.

@phansys
Copy link
Contributor Author

phansys commented May 22, 2021

But that's not true! The CI tests only two specific configurations. There's a huge (combinatorial, impossible to try them all) number of valid configurations. All matching versions of all dependencies, as well as of PHP itself, with yes/no for every optional PHP extension. Then there is the host machine to consider... it's architecture, how much RAM is available to PHP, what's in php.conf, etc.

I don't think testing a package against all the possible scenarios is impossible.
If there are some constraints not declared properly in composer.json or if some of the possible combinations (lower / highest deps) are not covered, we need to fix that situation: the missing or wrong constraints must be added / narrowed in composer.json and the missing build jobs must be added to the build matrix.
By instance, if this package depends optionally on the existence or the availability of some extension, both situations must be covered by the CI pipeline of this package. IMO, the final user must be able to rely on the quality of the package given the constraints it declares. So, if the package manager allows this package to be installed, that means all of the requirements in the target environment are complying with the package requirements.

IIUC, your perspective is about using this package through other dependency manager than Composer.
In that case, I guess that package manager must declare the constraints in the same way.

I'm talking only about the quality of the release tarball, which composer conflates with the thing to be copied into your webroot. A priori, having the documentation and tests in the release tarball can only add to that quality; not harm it. The documentation is useful to some users, and does no harm to others. The tests are useful to everyone IMO, and do no harm to others.

Tests are part of what you can be interested in before taking the decision of using some package, since it can help to be sure the quality is guaranteed.
The case of documentation maybe is a little bit different, but since the package managers I know are not able to leverage these markdown files in a help command or similar, the responsibility about the package documentation remains out of the scope of the package installation. In other words, you must read the documentation before the install process.
That said, I think the documentation files can be kept if there is a way for some package managers providing this package to use it.

The sane way to install the (any) library is to put one copy, managed by the system, under PHP's include_dir. The tests should be run and then deleted; i.e. not installed. The documentation goes wherever your OS puts documentation. This is what we do, and deleting the documentation and tests only makes things worse: users don't get the docs and can't run the tests, and we never had any of composer's problems in the first place, so nothing is gained in exchange.

Using the PHP's include_path is maybe a valid option for environments where only one application runs, or where you want to be responsible by yourself about what PHP must include every time it is executed, but that isn't a good option or the sanest way for projects that use its own platform-independent loading mechanism.

@orlitzky
Copy link
Contributor

I don't think testing a package against all the possible scenarios is impossible.

I'm sorry, but it's a mathematical fact. If there are N possible "yes" or "no" choices that can be made on the user's machine (like is PHP extension foo installed?), there are 2^N configurations that you have to test. For small values of N, testing them all becomes infeasible. Instead, you have to let the user run the tests on his own machine, which tests only a small number of configurations, but covers 100% of the ones that people want to use.

If there are some constraints not declared properly in composer.json or if some of the possible combinations (lower / highest deps) are not covered, we need to fix that situation: the missing or wrong constraints must be added / narrowed in composer.json and the missing build jobs must be added to the build matrix.
By instance, if this package depends optionally on the existence or the availability of some extension, both situations must be covered by the CI pipeline of this package.

As above, this is impossible. Count how many optional extensions PHP has, call it N, and try to launch 2^N CI instances.

The case of documentation maybe is a little bit different, but since the package managers I know are not able to leverage these markdown files in a help command or similar, the responsibility about the package documentation remains out of the scope of the package installation. In other words, you must read the documentation before the install process.
That said, I think the documentation files can be kept if there is a way for some package managers providing this package to use it.

Every Linux package manager installs the documentation for its packages in a somewhat-standard place. You don't need a special "help" command because the help file is a plain text file located at /usr/share/doc/<package-name>, regardless of the package.

@phansys
Copy link
Contributor Author

phansys commented May 25, 2021

As above, this is impossible. Count how many optional extensions PHP has, call it N, and try to launch 2^N CI instances.

I don't know why we should check this package against setups using extensions not declared as a dependency.

Every Linux package manager installs the documentation for its packages in a somewhat-standard place. You don't need a special "help" command because the help file is a plain text file located at /usr/share/doc/, regardless of the package.

That's fine for me. If it helps users using Linux package managers, I'll update the PR to keep the .md files.

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

Successfully merging this pull request may close these issues.

None yet

3 participants