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

Improve php-cs-fixer setup #185

Closed
wants to merge 6 commits into from

Conversation

yariksheptykin
Copy link
Contributor

@yariksheptykin yariksheptykin commented Mar 27, 2024

This PR:

  • adds cs-fixer as a dev dependency
  • registers add tests directory with the cs-fixer
  • executed cs-fixer within github actions

@yariksheptykin yariksheptykin changed the title Register tests dir with cs-fixer Improve cs-fixer setup Mar 27, 2024
@yariksheptykin yariksheptykin changed the title Improve cs-fixer setup Improve php-cs-fixer setup Mar 27, 2024
@mattamon
Copy link
Contributor

Hey @yariksheptykin thank you for your contribution.

Unfortunately we cannot accept it. The CS-Fixer rules are a company managed topic. Changing those rules needs to be considered for every bundle.

In such cases it is always better to open up a issue/discussion first, so we can evaluate the situation and give you feedback beforehand.

@mattamon mattamon self-assigned this Mar 29, 2024
@yariksheptykin
Copy link
Contributor Author

Hey @mattamon ! I don't understand what you mean. Do you mean that you at pimcore want to have a uniform cs-fixer ruleset that applies to all bundles and the skeleton? And you suggest to open issue first to duscuss which rules should go into the ruleset?

To clarify: my vision for this PR was to make use of the file that is already being shipped with the skeleton. I wanted to make sure that this file stays up to date, therefore I added cs-lint to CI. I also wanted to fix cs for the sources we ship with the skeleton. Whoever creates a new pimcore project from the skeleton should be able to quickly set up cs fixer, and the first commit should not be: "fixing cs in skeleton sources".

@yariksheptykin
Copy link
Contributor Author

@mattamon your PR #188 is a better option. Feel free to close this one. We can start an issue to collect reasonable CS rules, which we can share pimcore-wide if you think it makes sense.

@mattamon
Copy link
Contributor

mattamon commented Apr 5, 2024

@yariksheptykin Okay thank you! I will close your PR and merge mine.

It would be awesome if you could open up a discussion! https://github.com/orgs/pimcore/discussions

@mattamon mattamon closed this Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants