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

Upgrade to Symfony 7.2 #1532

Merged
merged 6 commits into from
Oct 3, 2024
Merged

Upgrade to Symfony 7.2 #1532

merged 6 commits into from
Oct 3, 2024

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Oct 1, 2024

This PR is based on #1529 with the following changes:

  • composer.lock is no committed, so that people always get the latest dependencies (note that this is how e.g. laravel installation works, so we're not inventing anything here, this is proven strategy.)
  • because composer.lock is not committed, flex will run recipes that are not found in symfony.lock. In this PR, I removed listing symfony/framework-bundle there, so that its recipe is run when installing the demo app. As a result and thanks to [symfony/framework-bundle] Generate APP_SECRET in .env.local recipes#1342, flex will generate APP_SECRET in .env.local
  • I also synced all recipes meanwhile

@nicolas-grekas nicolas-grekas changed the title Upgrade du Symfony 7.2 Upgrade to Symfony 7.2 Oct 1, 2024
@javiereguiluz
Copy link
Member

The only important requirement for this project is that it must be fully usable without doing anything (run no commands, change no config option, etc.)

A user runs symfony new --demo my_project (or composer create-project symfony/symfony-demo my_project) and then they have a fully usable project. If that it's true after all these changes, I'm OK with it.


In framework.yaml file, I think we need to readd this option: csrf_protection: true because the default value of that option is false.

@nicolas-grekas
Copy link
Member Author

csrf_protection should be enabled by default, so everything should work after a create-project

@javiereguiluz
Copy link
Member

OK, thanks. Can you please fix the linter issues? Then, I'll merge this. Thanks.

@stof
Copy link
Member

stof commented Oct 1, 2024

  • composer.lock is no committed, so that people always get the latest dependencies (note that this is how e.g. laravel installation works, so we're not inventing anything here, this is proven strategy.)

The symfony/demo package is not about doing a new symfony installation but about being a project showcasing the best practices. So I don't think this argument holds true.

@nicolas-grekas
Copy link
Member Author

Possibly @stof. The second argument still holds :)

@stof
Copy link
Member

stof commented Oct 1, 2024

your second argument made me realize a big shortcoming of your new Flex feature (I commented at symfony/flex#1028 (comment) as this belongs to Flex).
Actual projects won't be able to do your "hack" to fix it

@stof
Copy link
Member

stof commented Oct 1, 2024

Btw, your hack would be a pain to maintain as this manual removal will have to be re-applied each time someone contributes an update.

@nicolas-grekas
Copy link
Member Author

CI fixed @javiereguiluz
I also bumped twbs/bootstrap and symfonycasts/sass-bundle which were using outdated versions.

your hack would be a pain to maintain as this manual removal will have to be re-applied each time someone contributes an update.

@stof let's see. This should be manageable IMHO.

@stof
Copy link
Member

stof commented Oct 1, 2024

@nicolas-grekas it would be very easy for contributors to submit an updated symfony.lock that would be missed during code review (especially given it is collapsed by default).
And if we solve properly this problem space in Flex, we might not need this hack at all in the demo.

@nicolas-grekas
Copy link
Member Author

We need to be careful indeed.
Also we don't have a solution in flex, so I'm not sure what you have in mind. Can you expand on that?

@stof
Copy link
Member

stof commented Oct 1, 2024

Well, in Flex, you are suggesting to use .env.dev instead of .env.local

@nicolas-grekas
Copy link
Member Author

nicolas-grekas commented Oct 1, 2024

Vote pending BTW on that PR :)

But this PR on flex won't solve the issue you're raising about maintainability: we'll still want flex to generate a new secret when the demo is installed, and the only way to do so it to run the fwb recipe at installation time, aka not commit the recipe in symfony.lock.

@javiereguiluz
Copy link
Member

Thanks Nicolas!

@javiereguiluz javiereguiluz merged commit eba4f92 into main Oct 3, 2024
6 checks passed
@xabbuh xabbuh deleted the sf72 branch October 4, 2024 06:50
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