-
-
Notifications
You must be signed in to change notification settings - Fork 4
Add support for bootstrapping Sulu projects #43
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
base: master
Are you sure you want to change the base?
Add support for bootstrapping Sulu projects #43
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a big chunk of duplication with the original Symfony template (which makes totally sense) but should this mean partials should be introduced with the use of include
?
upsun/10-sulu-upsun.yaml
Outdated
{{- $hasVarnish := false -}} | ||
{{- range $service := $.Services -}} | ||
{{- if eq $service.Type "varnish" -}} | ||
{{- $hasVarnish = true -}} | ||
{{- end -}} | ||
{{- end -}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to merge the two loops over $.Services
and move routes
after services
(because keys order in the YAML does not matter)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to use a single initial loop to gather the needed information. I’d prefer to keep the original order in the YAML file for better DX and readability.
6918b43
to
023992d
Compare
@tucksaun could you provide an example of how |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please rebase and consider this change:
- "/var": { source: storage, source_path: var }
+ "/var/cache": { source: instance, source_path: var/cache }
+ "/var/cache/prod/pools/app": { source: storage, source_path: var/cache/prod/pools/app }
"/public/uploads": { source: storage, source_path: public/uploads }
- {{ if file_exists "templates/debug/source_code.html.twig" -}}
- "/data": { source: storage, source_path: "data" }
- {{- end }}
Also, I see many whitespace control have changed position.
I'd like this file to be as much in sync with the flex-one as possible.
Could you figure out how these differences could minimized?
About "include" as suggested by @tucksaun, this can be done later on.
cdf76c8
to
8b9e55d
Compare
03aa915
to
239172a
Compare
7e04f93
to
16a5003
Compare
16a5003
to
f685a3a
Compare
@nicolas-grekas ... are you sure these changes work properly on upsun? I’m getting a Otherwise, the template should be ready now. Since I’m not an upsun expert, it might make sense for someone with more experience to review and challenge the template. I did my best, but I have limited experience in this area. |
With the changes of symfony-cli/symfony-cli#645 and this new Sulu specific upsun template, it is now much easier to create and deploy a new Sulu project on upsun.
Examples:
Basic Sulu project:
symfony new sulu-beta --skeleton=sulu --version=3.0.0-beta-1 --upsun
With Varnish (requires additional config update in the skeleton)
symfony new sulu-beta --skeleton=sulu --version=3.0.0-beta-1 --upsun --service=varnish
Redis as session storage (to scale the application):
symfony new sulu-beta --skeleton=sulu --version=3.0.0-beta-1 --upsun --service=redissession:redis-persistent