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 .vscode directory, and add it to .gitignore #583

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

vlakoff
Copy link
Contributor

@vlakoff vlakoff commented Apr 27, 2024

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
Copy link
Collaborator

n0nag0n commented Apr 28, 2024

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

fadrian06 commented Apr 28, 2024

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 n0nag0n merged commit ad58c09 into flightphp:master Apr 29, 2024
@n0nag0n
Copy link
Collaborator

n0nag0n commented Apr 29, 2024

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 !

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

Successfully merging this pull request may close these issues.

3 participants