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
Extract Sage 9 compiler #1030
Extract Sage 9 compiler #1030
Conversation
9847bb9
to
b425baa
Compare
Might a good idea. As is right now, I don't like having I'm not sure how generic we can make this. Composer + Yarn install are common/generic. |
Instead of making compile steps (composer, yarn, copy, etc) generic, I suggest normalizing the way we complie frontend assets in deploy hooks; similar idea to #896 Reasons:
For example, this is what developers would do if they using both sage 8 and sage 9 themes on the same server. # requirements.yml
+ - src: XXX.trellis-sage-8-compiler
+ version: 1.2.3
+ - src: YYY.trellis-sage-9-compiler
+ version: 1.2.3 # group_vars/all/deploy-hooks.yml
+ deploy_build_before:
+ - "{{ playbook_dir }}/vendor/roles/XXX.trellis-sage-8-compiler/tasks/main.yml"
+ - "{{ playbook_dir }}/vendor/roles/YYY.trellis-sage-9-compiler/tasks/main.yml" # group_vars/<env>/wordpress_sites.yml
wordpress_sites:
example.com:
+ sage_8_compiler:
+ - my-sage-eight-theme
another-example.com:
+ sage_9_compiler:
+ - my-sage-nine-theme
multisite-example.com:
+ sage_9_compiler:
+ - my-sage-nine-theme
+ - my-second-sage-nine-theme
+ sage_8_compiler:
+ - my-sage-eight-theme
+ - my-second-sage-eight-theme same idea goes to Laravel mix
How about this:
|
Sounds like a good plan 👍 Only question is if it makes sense for the wp sites property |
I suggest not to make it generic to allow role authors to define whatever the structure(dict, array, string, boolean) they want. Only thing we expecting here is the property name should be unique. This is because we don't want to limit role authors' flexibility. From the forum, I see people want to use Trellis on various frondend frameworks and static sites generator(e.g: Vue on top of Laravel, GatsbyJS, etc). They might require a very different kind of config in Let me know if you want go for extracting sage compiler into a separate ansible galaxy role. |
Yeah that makes sense. Go for it 👍 |
b425baa
to
499bcca
Compare
Help wanted Although I extracted to trellis-sage-9-compiler, there are some more tests and todos awaiting help. See: https://github.com/ItinerisLtd/trellis-sage-9-compiler#todos-help-wanted Question:
|
Yeah I think we should remove all mentions of it, except for maybe an updated default Maybe Either way, we'd need to update our docs to include a page on Trellis + Sage I think. This would deserve more than an entry in the user contributed extensions page. @retlehs what do you think? |
499bcca
to
a0c5296
Compare
a0c5296
to
03d351e
Compare
Done. |
@retlehs had some thoughts before on this. Can you explain them? |
i would prefer to close this PR entirely and just add the sage 9 compiler to the user contributed roles docs for if they want to use it... we can't make it harder/more confusing to use sage out of the box with trellis |
Process now:
Process after:
Is that correct? Obviously it's more work, but it's done in a more extensible way. |
if we're going to do this, there needs to be a theme agnostic ansible role that we can include in trellis by default there shouldn't be anything specific to sage 8, sage 9, or even a theme - since plugins should be allowed to take advantage of this too ideally there's some simple way to configure this in trellis (maybe more inline with how a CI config looks?), but i don't have a better idea of what that could be right now |
Second attempt of #997 plus compile multiple sage themes
What's wrong with #997?
Technically nothing's wrong. However, changing from
project_local_path
toproject_build_path
confuses developers (#997 (comment), #997 (comment)). Using a different variable name is not enough to show the differences between those variables:project_local_path
: the local bedrock path, e.g:/path/to/trellis/../site/example1
, the one that you developing onproject_build_path
: the temporary build path, under$TMPDIR/trellis
or/tmp/trellis
, not respectingproject.repo_subtree_path
Thus, this attempt removes the needs of changing
deploy-hooks/build-before.yml
and requires develoeprs to add their sage theme names ingroup_vars/<env>/wordpress_sites.yml
:Why multiple sage theme? Or, In what cases it will be helpful?
Testing needed!!
We have been using it on production for a while with file structure described here in #883 (comment)
Never tested on the roots-offical-docs way.
Discussion: Ansible Galaxy Role?
Since
deploy-hooks/build-before.yml
is useless when you don't use sage and support for Laravel projects is planned #951, would it make sense if we extractbuild-before.yml
into a separate ansible galaxy role?cc: @codepuncher, @BufferOverNo, @JackAlexander1