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

[RFC] Command to remove non-production files #22064

Closed
linaori opened this issue Mar 20, 2017 · 20 comments
Closed

[RFC] Command to remove non-production files #22064

linaori opened this issue Mar 20, 2017 · 20 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@linaori
Copy link
Contributor

linaori commented Mar 20, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version latest

I would like to propose a new feature that can be bundled with Symfony: A command that can remove files that you don't need in production. There have been several suggestions in the past to not include things like tests when developing:

I envision this as a command that people can add to their composer.json to remove:

  • Test files/directories
  • Docs (including images)
  • md files (readme, upgrade, etc), might have to be optional

This should definitely not be removing source files like Tree Shaking! You can narrow your dependencies for that to specific components/bundles.

Several use-cases could be satisfied by this:

  • Reducing the effective size of the application when using a buildserver or bundling Symfony in a desktop app /cc @jrobeson
  • A clean environment when developing, no mocks or test files in auto-completion
  • No readme files, examples or upgrade logs/contribution logs showing up in search results (hence this step might have to be optional).
@javiereguiluz
Copy link
Member

javiereguiluz commented Mar 20, 2017

I like this idea ... but I don't like how specific the proposed command is. Instead of production:remove-files command, I'd create a production:prepare command. The tasks that this command could do are:

  • "Compile" the application: warm-up and generate everything that has to be generated to run the Symfony app. And do that in a directory-independent manner, so we can prepare the app in one dir and then make the switch to another dir and start serving requests right away.
  • "Optimize" the application: the most obvious thing would be to remove all the useless files as suggested by @iltar. In the future, it could be more "intelligent" and do other safe optimizations.
  • etc.

All these tasks could be enabled/disabled via command options.

@linaori
Copy link
Contributor Author

linaori commented Mar 20, 2017

Regardless of details, this would open up other possibilities:

  • a way of building your application (possibly with custom scripts)
  • a way of adding sanity checks (like are all my paths relative, including in vendor)
  • a way of possibly even deploying your application to a server (possibly with custom scripts)

I know that this most likely won't be an advisable way of deployment for bigger companies or companies that have multiple people working on applications. Though, this could be fixed with custom build scripts. However, this could make deployments conceptually a lot easier for people that have no fancy deployment tools or server. Think of:

  • Deployment scripts for shared hosting, which a hosting provider could provide to customers (my situation)
  • Deployment scripts that could be shared via github for example
  • Integration with existing deployment tools

@javiereguiluz
Copy link
Member

I'd say that anything related to deployment is a no-go for being too complex and too hard to do it right for a large enough number of use cases.

@javiereguiluz javiereguiluz added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Mar 20, 2017
@craigh
Copy link

craigh commented Mar 20, 2017

I like this idea.

@rvanlaak
Copy link
Contributor

Would be great if Composer could handle removing all these files when --prefer-dist is passed.

@steevanb
Copy link

steevanb commented Apr 2, 2017

To me, that's not Symfony purpose, it's deploy script purpose.

@linaori
Copy link
Contributor Author

linaori commented Aug 22, 2017

Just encountered this DX issue:

image

It's still extremely annoying that all Symfony test files are available when develop. Not only does it add a significant amount of classes to auto completion, it also creates confusion as to which class you actually need.

@Taluu
Copy link
Contributor

Taluu commented Aug 22, 2017

IMO this is related to PHPStorm which is aggressively checking all files...

Or maybe when composer installs stuff, we should exclude non-dev directories. But that would be a job for composer.

@nicolas-grekas
Copy link
Member

Can't this be handled by the phpstorm plugin?

@theofidry
Copy link
Contributor

@iltar if I have to look at the vendor (source & test) code, I need them to be indexed by PHPStorm so excluding them is not a solution. It's more the auto-complete of PHPStorm that needs to be smarter, it's like when I'm starting to type a $a for an argument and it suggests me $argc first which is obviously wrong. But in any case that is more a PHPStorm issue than Symfony, either in PHPStorm itself or the plugin

@linaori
Copy link
Contributor Author

linaori commented Aug 22, 2017

The issue is not the IDE. This issue is not that the plugin doesn't exclude this. The problem is that Symfony as dependency comes with test files which are 100% irrelevant to development when using Symfony as framework. I don't see how test files are relevant to using the framework.

Was playing around with the WebTestCase just now and I already picked the wrong class by accident.

image

If you have to look at the tests, look at github or make a clone/checkout of the symfony repository. As developer, you're only interested in the source code, not the tests. The current Developer Experience is that all relevant test classes show up in my auto complete, simply because they exist in the dist version.

@theofidry
Copy link
Contributor

The problem is that Symfony as dependency comes with test files which are 100% irrelevant to development when using Symfony as framework [...] If you have to look at the tests, look at github or make a clone/checkout of the symfony repository. As developer, you're only interested in the source code, not the tests

I disagree. I often consider tests are part of the documentation: they are helpful to understand how the code works and what is the current behaviour when the source code is not clear enough

@linaori
Copy link
Contributor Author

linaori commented Aug 22, 2017

I highly disagree with that. There's literally 0 cases where I've ever found the tests of a package useful when they have mocked out everything. If you want documentation, the code should usually be sufficient. If you need documentation on how to use something, it should be documented in the docs. If this is not the case, it should be added, rather than cluttering everyone with files they will most likely never need.

Worst case scenario, you can still look it up in the Symfony repository on either github or in your local clone. Could hold a poll to see who think it's not needed to have the files vs those who actually use the files to view how it's used.

I don't think that the majority of developers should be bothered by a shortcut for the few.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Aug 22, 2017

Removing non production files - and removing entries from autocompletion - are two totally different issues.
The fact that one implies the other is not an argument that justifies removing non-production files.
Disagreeing is fine, but moving forward is better :)

Here, the target of not having tests be proposed while autocompleting is a nice one. I would even like it when working on Symfony itself. Which is a use case that is not covered by this RFC btw.

On the autocompletion topic, I'm sticking to my pov expressed in #17749: editorconfig/editorconfig#228 is the correct solution, and meanwhile, the Symfony phpStorm plugin could help.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Aug 22, 2017

I just submitted this as a suggestion to phpStorm (you might do the same if you like it :) ):

When a PHP class or method is tagged @internal and @deprecated, it is still suggested when using autocompletion.
If one validate the suggestion, one gets the selected method/class, with a line through (strike), hinting this should not be used.

But why suggest a non-recommended method/class in the first place?
This is rather annoying as many of us make wrong selection by mistake.

It would be great if phpStorm could at least suggest these entries last in the list.
Even having them removed would be OK for me - or maybe have them suggested only if no other choice exists?

That's definitely something people a struggling with, I hope you might do something to enhance the UX.

@linaori
Copy link
Contributor Author

linaori commented Aug 22, 2017

I believe there's already an issue open (if not multiple) about the internal

@theofidry
Copy link
Contributor

editorconfig/editorconfig#228 looks by far the most promising one to me. You could always PHPStorm to try to not put priority in the files found in the tests folder for auto-completion but then they will have trouble for when tests are in fancy locations and it's not just src & tests

@Haehnchen
Copy link
Contributor

about autocompletion: just open an issue at JetBrains. i am also struggling in completion window but not only in Symfony projects.

there are ways to help with the Symfony plugin but a generic solution would be more helpful

@nicolas-grekas
Copy link
Member

Here is the phpStorm issue if you want to chime in:
https://youtrack.jetbrains.com/issue/WI-37750

@linaori
Copy link
Contributor Author

linaori commented Apr 19, 2018

Closing this issue as it's not a good idea

@linaori linaori closed this as completed Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Development

No branches or pull requests

9 participants