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

add symfony/flex to require-dev #5346

Merged

Conversation

alexislefebvre
Copy link
Contributor

@alexislefebvre alexislefebvre commented Jul 25, 2022

Follow-up of #5330

There's no need to require symfony/flex on the CI, it can be added to required-dev.

This is the same idea than symfony/symfony-docs#17031

@michaelKaefer
Copy link
Contributor

@alexislefebvre Isn't there a small difference?

  • code depending on a package -> use required-dev to be able to develop
  • CI depending on package -> use CI configuration to install it

(What do you mean with #17031 ?)

@alexislefebvre
Copy link
Contributor Author

alexislefebvre commented Aug 5, 2022

@michaelKaefer this is right, I propose to require it so that it doesn't need to be written 7 times for the workflows.

I fixed the link.

@michaelKaefer
Copy link
Contributor

@alexislefebvre Thanks for the link, ok I see, this PR could be merged if it becomes best practice.

@javiereguiluz javiereguiluz merged commit 12380b7 into EasyCorp:4.x Aug 5, 2022
@alexislefebvre alexislefebvre deleted the add-symfony/flex-to-require-dev branch August 5, 2022 17:23
@javiereguiluz
Copy link
Collaborator

I don't have a strong opinion about this ... but I've decided to merge it. Please note that I moved the symfony/flex dependency from require to require-dev while merging.

@alexislefebvre
Copy link
Contributor Author

@alexislefebvre Isn't there a small difference?

* code depending on a package -> use `required-dev` to be able to develop

* CI depending on package -> use CI configuration to install it

To address your remark: this is true, yet I think there's no downside to require it as a dev dependency.


I don't have a strong opinion about this ... but I've decided to merge it. Please note that I moved the symfony/flex dependency from require to require-dev while merging.

Oops! I screwed up, thanks for fixing it.

@alexislefebvre
Copy link
Contributor Author

To address your remark: this is true, yet I think there's no downside to require it as a dev dependency.

I was so wrong, symfony/flex update the recipes and it create config files that we don't want in this context, please see #5354

javiereguiluz added a commit that referenced this pull request Aug 6, 2022
…bvre)

This PR was squashed before being merged into the 4.x branch.

Discussion
----------

Remove symfony/flex from composer require dev

With the changes from #5346, cloning this project and running `composer install` will run the recipes from Symfony, it will add many files that we don't want:

```bash
$ composer install
[…]
$ git status
On branch 4.x
Your branch is up to date with 'origin/4.x'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   .gitignore
	modified:   composer.json
	modified:   phpunit.xml.dist

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	.env
	.env.test
	bin/
	config/
	docker-compose.override.yml
	docker-compose.yml
	public/
	src/Controller/.gitignore
	src/DataFixtures/
	src/Entity/
	src/Kernel.php
	src/Repository/
	symfony.lock
	templates/
	translations/
```

This PR revert this.

I apologize, I should have tested that before assuming that `symfony/flex` would not intervene.

Commits
-------

3131083 Remove symfony/flex from composer require dev
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.

None yet

4 participants