Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpFoundation] optimize files for distribution#24427
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Closed
Closed
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Contributor
Tobion commentedOct 4, 2017
Author
jpuck commentedOct 4, 2017
@Tobion ah thanks for the reading material. So should I just close now then? |
Member
javiereguiluz commentedOct 5, 2017
@jpuck it's true that this has been asked for (and rejected) many times before, but it's OK to discuss about this again from time to time. Besides, if so many people and so many times ask for the same thing, maybe it's time to reconsider our past decision. Thanks! |
Contributor
linaori commentedOct 5, 2017
I'm still of opinion that it's 100% useless to include tests unless you specifically install the source as dependency. I don't care about the amount of files, or even that it's in production, but I do care about the DX of developing with Symfony. Current DX status when looking up classes and implementations:
The biggest issue is thatall searches are heavily polluted with search results from tests:
While everything is being done to enhance the DX of Symfony, this is still a big issue. |
Contributor
Tobion commentedOct 5, 2017
I full agree wit@iltar and I've alwas been in favor in adding this. |
Contributor
sroze commentedOct 5, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Definitely agree with this as well, these tests files shouldn't be within the "distributed archive". Ideally I'd move them to |
Contributor
linaori commentedOct 5, 2017
The problem is primarily present when you use |
Member
nicolas-grekas commentedOct 5, 2017
Having tests in each component is a requirement so there is no way to move tests outside of src, unless someone wants to take the responsibility and has the leadership to rewrite all our processes and releasing infrastructure. About DX, I thinkhttps://youtrack.jetbrains.com/issue/WI-37750 will do the job. 👎 on my side, with the same rationale as previously detailed. |
Contributor
afurculita commentedOct 5, 2017
For |
Contributor
linaori commentedOct 5, 2017
That means all tests would have to be marked with |
Member
nicolas-grekas commentedOct 5, 2017
@iltar yes. That looks accurate to me, anything in "Tests" folder is out of BC policy, so they are internal. |
Contributor
linaori commentedOct 5, 2017
But that won't fix it for:
|
Member
nicolas-grekas commentedOct 5, 2017
It's still an IDE-domain problem. |
Contributor
linaori commentedOct 5, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Excluding the tests from dist files would cover all scenarios though, no opt-outs for anything, no additional work for the developer |
Member
nicolas-grekas commentedOct 5, 2017
Not all usages, as previously discussed in#17749. |
Contributor
linaori commentedOct 5, 2017
That was a slightly different topic, moving the files. The only thing I would like to see this issue resolve in, is no test files present when I add symfony as a dependency to my project. I have yet to see an argument that might convince me that this is a bad idea for the majority of people. Adjusting the IDE or config files for every single developer is a lot of work, where as the minuscule amount of developers thatdoes want the test files, can get them via numerous ways. Why not hold a poll whether or not the community wants the test files included or excluded from the dist? |
Member
nicolas-grekas commentedOct 8, 2017
Closing as explained. |
fabpot added a commit that referenced this pull requestSep 16, 2019
…dist" (Nyholm)This PR was merged into the 4.4 branch.Discussion----------Adding .gitattributes to remove Tests directory from "dist"| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no?| Tickets || License | MIT| Doc PR |This is a controversial topic that have been mentioned before. We recently had some discussions on Slack about it and the community not in an agreement. This was asked back in 2014 already.Im making this PR again, because I think this will help more people than it hurts to keep the tests in the "dist" version.### Reasons for keeping the tests with the source* You can look at the tests to understand how the code works* It is convenientIn the past there were an argument of people might depend on Symfony's classes in Tests. That is no longer the case since we moved reusable classes from Tests to Test.### Reasons for removing them (merging this PR)* There should be difference between `composer update --prefer-source` and `composer update --prefer-dist`* Smaller packages when deploying with Docker or on Serverless.* Static analysis tools will not complain on PHP syntax errors in our tests ([example](https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xml_with_wrong_ext.php))## How to decide?Merging this PR or not is tricky because no side has a solid technical argument. It is basically just personal preference. Please give this PR a 👍 or 👎 if you want to give your opinion.## Other PRs and issues related to this:Add .gitattributes file (#29277)Added .gitattributes files to root and all components (#26472)Exclude non-essential files from Composer package (#25414)[HttpFoundation] optimize files for distribution (#24427)Add .gitattributes files (#23926)[Suggestion] Adding .gitattributes to ignore unnecessary folders and files for production env (#20057)Add lightweight and root only .gitattributes (#18004)Add .gitattributes to exclude tests from ZIPs (#17995)[RFC] Move tests out of the source and source out of the tests (#17749)Removal of development & testing files using .gitattributes (#16174)Please add .gitattributes files and fix line endings (#13521)making use of .gitattributes (#11810)## WorkaroundsThere are workarounds for both sides. Example:### Workaround if merged* `composer update --prefer-source`### Workaround if closed* `find vendor/symfony -name "Tests" -type d -exec rm -r "{}" \;`*editorconfig/editorconfig#228*https://github.com/dg/composer-cleanerCommits-------ac7dc24 Adding .gitattributes to remove Tests directory from "dist"
symfony-splitter pushed a commit to symfony/mailer that referenced this pull requestSep 16, 2019
…dist" (Nyholm)This PR was merged into the 4.4 branch.Discussion----------Adding .gitattributes to remove Tests directory from "dist"| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no?| Tickets || License | MIT| Doc PR |This is a controversial topic that have been mentioned before. We recently had some discussions on Slack about it and the community not in an agreement. This was asked back in 2014 already.Im making this PR again, because I think this will help more people than it hurts to keep the tests in the "dist" version.### Reasons for keeping the tests with the source* You can look at the tests to understand how the code works* It is convenientIn the past there were an argument of people might depend on Symfony's classes in Tests. That is no longer the case since we moved reusable classes from Tests to Test.### Reasons for removing them (merging this PR)* There should be difference between `composer update --prefer-source` and `composer update --prefer-dist`* Smaller packages when deploying with Docker or on Serverless.* Static analysis tools will not complain on PHP syntax errors in our tests ([example](https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xml_with_wrong_ext.php))## How to decide?Merging this PR or not is tricky because no side has a solid technical argument. It is basically just personal preference. Please give this PR a 👍 or 👎 if you want to give your opinion.## Other PRs and issues related to this:Add .gitattributes file (symfony/symfony#29277)Added .gitattributes files to root and all components (symfony/symfony#26472)Exclude non-essential files from Composer package (symfony/symfony#25414)[HttpFoundation] optimize files for distribution (symfony/symfony#24427)Add .gitattributes files (symfony/symfony#23926)[Suggestion] Adding .gitattributes to ignore unnecessary folders and files for production env (symfony/symfony#20057)Add lightweight and root only .gitattributes (symfony/symfony#18004)Add .gitattributes to exclude tests from ZIPs (symfony/symfony#17995)[RFC] Move tests out of the source and source out of the tests (symfony/symfony#17749)Removal of development & testing files using .gitattributes (symfony/symfony#16174)Please add .gitattributes files and fix line endings (symfony/symfony#13521)making use of .gitattributes (symfony/symfony#11810)## WorkaroundsThere are workarounds for both sides. Example:### Workaround if merged* `composer update --prefer-source`### Workaround if closed* `find vendor/symfony -name "Tests" -type d -exec rm -r "{}" \;`*editorconfig/editorconfig#228*https://github.com/dg/composer-cleanerCommits-------ac7dc24bcb Adding .gitattributes to remove Tests directory from "dist"
symfony-splitter pushed a commit to symfony/web-profiler-bundle that referenced this pull requestSep 16, 2019
…dist" (Nyholm)This PR was merged into the 4.4 branch.Discussion----------Adding .gitattributes to remove Tests directory from "dist"| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no?| Tickets || License | MIT| Doc PR |This is a controversial topic that have been mentioned before. We recently had some discussions on Slack about it and the community not in an agreement. This was asked back in 2014 already.Im making this PR again, because I think this will help more people than it hurts to keep the tests in the "dist" version.### Reasons for keeping the tests with the source* You can look at the tests to understand how the code works* It is convenientIn the past there were an argument of people might depend on Symfony's classes in Tests. That is no longer the case since we moved reusable classes from Tests to Test.### Reasons for removing them (merging this PR)* There should be difference between `composer update --prefer-source` and `composer update --prefer-dist`* Smaller packages when deploying with Docker or on Serverless.* Static analysis tools will not complain on PHP syntax errors in our tests ([example](https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xml_with_wrong_ext.php))## How to decide?Merging this PR or not is tricky because no side has a solid technical argument. It is basically just personal preference. Please give this PR a 👍 or 👎 if you want to give your opinion.## Other PRs and issues related to this:Add .gitattributes file (symfony/symfony#29277)Added .gitattributes files to root and all components (symfony/symfony#26472)Exclude non-essential files from Composer package (symfony/symfony#25414)[HttpFoundation] optimize files for distribution (symfony/symfony#24427)Add .gitattributes files (symfony/symfony#23926)[Suggestion] Adding .gitattributes to ignore unnecessary folders and files for production env (symfony/symfony#20057)Add lightweight and root only .gitattributes (symfony/symfony#18004)Add .gitattributes to exclude tests from ZIPs (symfony/symfony#17995)[RFC] Move tests out of the source and source out of the tests (symfony/symfony#17749)Removal of development & testing files using .gitattributes (symfony/symfony#16174)Please add .gitattributes files and fix line endings (symfony/symfony#13521)making use of .gitattributes (symfony/symfony#11810)## WorkaroundsThere are workarounds for both sides. Example:### Workaround if merged* `composer update --prefer-source`### Workaround if closed* `find vendor/symfony -name "Tests" -type d -exec rm -r "{}" \;`*editorconfig/editorconfig#228*https://github.com/dg/composer-cleanerCommits-------ac7dc24bcb Adding .gitattributes to remove Tests directory from "dist"
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull requestSep 16, 2019
…dist" (Nyholm)This PR was merged into the 4.4 branch.Discussion----------Adding .gitattributes to remove Tests directory from "dist"| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no?| Tickets || License | MIT| Doc PR |This is a controversial topic that have been mentioned before. We recently had some discussions on Slack about it and the community not in an agreement. This was asked back in 2014 already.Im making this PR again, because I think this will help more people than it hurts to keep the tests in the "dist" version.### Reasons for keeping the tests with the source* You can look at the tests to understand how the code works* It is convenientIn the past there were an argument of people might depend on Symfony's classes in Tests. That is no longer the case since we moved reusable classes from Tests to Test.### Reasons for removing them (merging this PR)* There should be difference between `composer update --prefer-source` and `composer update --prefer-dist`* Smaller packages when deploying with Docker or on Serverless.* Static analysis tools will not complain on PHP syntax errors in our tests ([example](https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xml_with_wrong_ext.php))## How to decide?Merging this PR or not is tricky because no side has a solid technical argument. It is basically just personal preference. Please give this PR a 👍 or 👎 if you want to give your opinion.## Other PRs and issues related to this:Add .gitattributes file (symfony/symfony#29277)Added .gitattributes files to root and all components (symfony/symfony#26472)Exclude non-essential files from Composer package (symfony/symfony#25414)[HttpFoundation] optimize files for distribution (symfony/symfony#24427)Add .gitattributes files (symfony/symfony#23926)[Suggestion] Adding .gitattributes to ignore unnecessary folders and files for production env (symfony/symfony#20057)Add lightweight and root only .gitattributes (symfony/symfony#18004)Add .gitattributes to exclude tests from ZIPs (symfony/symfony#17995)[RFC] Move tests out of the source and source out of the tests (symfony/symfony#17749)Removal of development & testing files using .gitattributes (symfony/symfony#16174)Please add .gitattributes files and fix line endings (symfony/symfony#13521)making use of .gitattributes (symfony/symfony#11810)## WorkaroundsThere are workarounds for both sides. Example:### Workaround if merged* `composer update --prefer-source`### Workaround if closed* `find vendor/symfony -name "Tests" -type d -exec rm -r "{}" \;`*editorconfig/editorconfig#228*https://github.com/dg/composer-cleanerCommits-------ac7dc24bcb Adding .gitattributes to remove Tests directory from "dist"
symfony-splitter pushed a commit to symfony/cache that referenced this pull requestSep 16, 2019
…dist" (Nyholm)This PR was merged into the 4.4 branch.Discussion----------Adding .gitattributes to remove Tests directory from "dist"| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no?| Tickets || License | MIT| Doc PR |This is a controversial topic that have been mentioned before. We recently had some discussions on Slack about it and the community not in an agreement. This was asked back in 2014 already.Im making this PR again, because I think this will help more people than it hurts to keep the tests in the "dist" version.### Reasons for keeping the tests with the source* You can look at the tests to understand how the code works* It is convenientIn the past there were an argument of people might depend on Symfony's classes in Tests. That is no longer the case since we moved reusable classes from Tests to Test.### Reasons for removing them (merging this PR)* There should be difference between `composer update --prefer-source` and `composer update --prefer-dist`* Smaller packages when deploying with Docker or on Serverless.* Static analysis tools will not complain on PHP syntax errors in our tests ([example](https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xml_with_wrong_ext.php))## How to decide?Merging this PR or not is tricky because no side has a solid technical argument. It is basically just personal preference. Please give this PR a 👍 or 👎 if you want to give your opinion.## Other PRs and issues related to this:Add .gitattributes file (symfony/symfony#29277)Added .gitattributes files to root and all components (symfony/symfony#26472)Exclude non-essential files from Composer package (symfony/symfony#25414)[HttpFoundation] optimize files for distribution (symfony/symfony#24427)Add .gitattributes files (symfony/symfony#23926)[Suggestion] Adding .gitattributes to ignore unnecessary folders and files for production env (symfony/symfony#20057)Add lightweight and root only .gitattributes (symfony/symfony#18004)Add .gitattributes to exclude tests from ZIPs (symfony/symfony#17995)[RFC] Move tests out of the source and source out of the tests (symfony/symfony#17749)Removal of development & testing files using .gitattributes (symfony/symfony#16174)Please add .gitattributes files and fix line endings (symfony/symfony#13521)making use of .gitattributes (symfony/symfony#11810)## WorkaroundsThere are workarounds for both sides. Example:### Workaround if merged* `composer update --prefer-source`### Workaround if closed* `find vendor/symfony -name "Tests" -type d -exec rm -r "{}" \;`*editorconfig/editorconfig#228*https://github.com/dg/composer-cleanerCommits-------ac7dc24bcb Adding .gitattributes to remove Tests directory from "dist"
symfony-splitter pushed a commit to symfony/yaml that referenced this pull requestSep 16, 2019
…dist" (Nyholm)This PR was merged into the 4.4 branch.Discussion----------Adding .gitattributes to remove Tests directory from "dist"| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no?| Tickets || License | MIT| Doc PR |This is a controversial topic that have been mentioned before. We recently had some discussions on Slack about it and the community not in an agreement. This was asked back in 2014 already.Im making this PR again, because I think this will help more people than it hurts to keep the tests in the "dist" version.### Reasons for keeping the tests with the source* You can look at the tests to understand how the code works* It is convenientIn the past there were an argument of people might depend on Symfony's classes in Tests. That is no longer the case since we moved reusable classes from Tests to Test.### Reasons for removing them (merging this PR)* There should be difference between `composer update --prefer-source` and `composer update --prefer-dist`* Smaller packages when deploying with Docker or on Serverless.* Static analysis tools will not complain on PHP syntax errors in our tests ([example](https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xml_with_wrong_ext.php))## How to decide?Merging this PR or not is tricky because no side has a solid technical argument. It is basically just personal preference. Please give this PR a 👍 or 👎 if you want to give your opinion.## Other PRs and issues related to this:Add .gitattributes file (symfony/symfony#29277)Added .gitattributes files to root and all components (symfony/symfony#26472)Exclude non-essential files from Composer package (symfony/symfony#25414)[HttpFoundation] optimize files for distribution (symfony/symfony#24427)Add .gitattributes files (symfony/symfony#23926)[Suggestion] Adding .gitattributes to ignore unnecessary folders and files for production env (symfony/symfony#20057)Add lightweight and root only .gitattributes (symfony/symfony#18004)Add .gitattributes to exclude tests from ZIPs (symfony/symfony#17995)[RFC] Move tests out of the source and source out of the tests (symfony/symfony#17749)Removal of development & testing files using .gitattributes (symfony/symfony#16174)Please add .gitattributes files and fix line endings (symfony/symfony#13521)making use of .gitattributes (symfony/symfony#11810)## WorkaroundsThere are workarounds for both sides. Example:### Workaround if merged* `composer update --prefer-source`### Workaround if closed* `find vendor/symfony -name "Tests" -type d -exec rm -r "{}" \;`*editorconfig/editorconfig#228*https://github.com/dg/composer-cleanerCommits-------ac7dc24bcb Adding .gitattributes to remove Tests directory from "dist"
symfony-splitter pushed a commit to symfony/mailer that referenced this pull requestSep 28, 2021
…dist" (Nyholm)This PR was merged into the 4.4 branch.Discussion----------Adding .gitattributes to remove Tests directory from "dist"| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | no| New feature? | yes| Deprecations? | no?| Tickets || License | MIT| Doc PR |This is a controversial topic that have been mentioned before. We recently had some discussions on Slack about it and the community not in an agreement. This was asked back in 2014 already.Im making this PR again, because I think this will help more people than it hurts to keep the tests in the "dist" version.### Reasons for keeping the tests with the source* You can look at the tests to understand how the code works* It is convenientIn the past there were an argument of people might depend on Symfony's classes in Tests. That is no longer the case since we moved reusable classes from Tests to Test.### Reasons for removing them (merging this PR)* There should be difference between `composer update --prefer-source` and `composer update --prefer-dist`* Smaller packages when deploying with Docker or on Serverless.* Static analysis tools will not complain on PHP syntax errors in our tests ([example](https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/xml_with_wrong_ext.php))## How to decide?Merging this PR or not is tricky because no side has a solid technical argument. It is basically just personal preference. Please give this PR a 👍 or 👎 if you want to give your opinion.## Other PRs and issues related to this:Add .gitattributes file (symfony/symfony#29277)Added .gitattributes files to root and all components (symfony/symfony#26472)Exclude non-essential files from Composer package (symfony/symfony#25414)[HttpFoundation] optimize files for distribution (symfony/symfony#24427)Add .gitattributes files (symfony/symfony#23926)[Suggestion] Adding .gitattributes to ignore unnecessary folders and files for production env (symfony/symfony#20057)Add lightweight and root only .gitattributes (symfony/symfony#18004)Add .gitattributes to exclude tests from ZIPs (symfony/symfony#17995)[RFC] Move tests out of the source and source out of the tests (symfony/symfony#17749)Removal of development & testing files using .gitattributes (symfony/symfony#16174)Please add .gitattributes files and fix line endings (symfony/symfony#13521)making use of .gitattributes (symfony/symfony#11810)## WorkaroundsThere are workarounds for both sides. Example:### Workaround if merged* `composer update --prefer-source`### Workaround if closed* `find vendor/symfony -name "Tests" -type d -exec rm -r "{}" \;`*editorconfig/editorconfig#228*https://github.com/dg/composer-cleanerCommits-------ac7dc24bcb Adding .gitattributes to remove Tests directory from "dist"
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently every production installation will pull in all of the tests into the vendor directory. This is unnecessary file bloat on the server and network bandwidth. By adding a
.gitattributesfile with filenames listed to be excluded during export, this will speed up installations and facilitate better stewardship of server resources.This is my 1st PR, so I hope I understood the contributing guidelines and submitted to the correct repository and branch, so please kindly advise if otherwise. If acceptable in principle, then I'd be happy to duplicate the efforts in additional commits for the other components. I chose the HttpFoundation subtree because it came to my attention as it was used in a project when a lint failed on a php 5.6 server only because of php 7 language used in one of the tests.
Thanks!