flightphp / core

An extensible micro-framework for PHP
https://docs.flightphp.com
MIT License
2.64k stars 408 forks source link

Remove .vscode directory, and add it to .gitignore #583

Closed vlakoff closed 7 months ago

vlakoff commented 7 months ago

Delete the .vscode directory, that had been reintroduced during #547, and add it back to the .gitignore file.

Helpful documentation: Git - gitignore Documentation, in particular the Pattern Format section.

By the way, excluding these IDE directories seems to be unpopular, because it should be the user's responsibility to exclude the metadata from the editors he is using.

See for instance:

However, Taylor Otwell ended up adding these exclude lines, and I think the added convenience is worth these few extra lines.

n0nag0n commented 7 months ago

So first off thanks for contributing! Usually we have a convo before a PR is submitted but this is small enough we can just have that convo here.

Originally it was excluded, but then it was included again because we wanted to keep workspace settings consistent among the maintainers and contributors while we were working out some consistent settings to be ran by automated processes (like spacing, whitespace, linting, etc).

So we do have better settings in place now. I'm curious what @fadrian06 thinks.

One thing to point out, Laravel is a fantastic framework, and has done a lot of good for the community! That being said however, we can learn from and in some cases mimic what Laravel does, but we shouldn't just blindly do it because Taylor did it. What Laravel is doing makes sense for Laravel, and it might make sense for Flight. Not a dig at you personally, just pointing it out for anyone who sees this PR.

fadrian06 commented 7 months ago

Reading a little about the references attached in the PR, I was struck by the "what if tomorrow there is another IDE?"... You just have to look at the numbers to realize that VSCode will not be a dead thing tomorrow.

I'm not against removing editor settings in the project, but like @n0nag0n I'm not against removing editor settings in the project, but like you mention @n0nag0n It was just a mechanism so that contributors always had the same spacing, end of line and so on... I personally tend to forget and realize very late before committing.

I support using SublimeText :D but I removed its configuration from the project because I think I'm the only one who uses it... but in the case of VSCode it's different

fadrian06 commented 7 months ago

The settings.json of .vscode will not even weigh 5kb, and it is not included when the package is installed, making Flight install very quickly even on slow connections...

Since we have the tests ignored in the composer install, we had the freedom to add more tests as necessary, the same happens with .vscode, which well only has the basics of code style, this way we ensure consistency in the PRs

fadrian06 commented 7 months ago

If you don't mind @n0nag0n and you will have your local configuration correct, removing other KB from the repository and making the flight lighter hehe

n0nag0n commented 7 months ago

I think I'm ok merging this because we have PHPCS which will make the commits and things more consistent and we don't need editor settings to be saved in the repo anymore. Thanks for the help @vlakoff !