Removed context from read_template_file, fixed tests to match new arity #441
Conversation
| @@ -138,18 +138,6 @@ def read_template_file(template_path, context) | |||
|
|
|||
| end | |||
|
|
|||
| def test_backwards_compatability_support_for_overridden_read_template_file | |||
There was a problem hiding this comment.
Did you remove this test because it was failing? Does that mean this PR break backwards compatibility?
There was a problem hiding this comment.
I dropped it because I removed the backward compatibility support in the include tag. Yes.
We'd like to drop support for both arities to make adding the caching less hacky.
|
Please review Thanks |
|
The changes look good to me, but we need to be careful when releasing this as a gem to respect semantic versioning. @arthurnn, do you think we should sneak this into 3.0.0? |
|
👍 If we don't want to sneak this into liquid 3, then we will want to start a liquid 4 branch where we can start putting following backwards incompatible changes. |
|
You only have an RC out and the SemVer spec doesn't seem to mention anything about breaking changes between RC and the actual release. 💛 yolo. |
|
In that case, add an entry to History.md then I say |
|
@sunblaze can you rebase and ship this? |
sunblaze
force-pushed
the
remove_context_from_read_template_file
branch
from
cbd1a07
to
9dd157c
Compare
|
Rebased. It's been a while so I'm not sure what needs to be added in the History.md for these changes. |
|
Test failure fixed in master. |
sunblaze
force-pushed
the
remove_context_from_read_template_file
branch
from
9dd157c
to
f0d862d
Compare
|
@sunblaze can this be shipped? can you own it? |
|
Yep, I'll own it! |
sunblaze
force-pushed
the
remove_context_from_read_template_file
branch
from
f0d862d
to
4592afc
Compare
|
Updated History.md and removed a couple more instances of the old method signature being used. |
|
🚢 🇮🇹 |
|
👍 |
…e_file Removed context from read_template_file, fixed tests to match new arity
|
@sunblaze, just wonder a little bit. Previously, I depend on this context variable to query current rails controller name, then I can build a template path variable which prefixed controller_path = context.registers[:controller].controller_path
template_path = "#{controller_path}/#{template_path}" unless template_path.include?('/') |
* Pin some dependencies to versions that support Ruby 2.5 or CI on Ruby 2.5 fails to run https://github.com/middleman/middleman/actions/runs/7052178976/job/19196687517#step:3:38 * Pin slim version to 4.x for now for the tests to pass * Delete Gemfile.lock before running bundle install in CI because dependent gem versions may differ between Ruby versions * ✂️ * setup-ruby already runs bundle install * The `HandleExceptions` cop has been renamed to `Lint/SuppressedException` * rubocop -A --only Style/StringConcatenation * Avoid unnecessary use of BasicObject or some tests would fail with the following error with recent versions of contract gem: can't convert Middleman::CoreExtensions::Collections::LazyCollectorStep to String (Middleman::CoreExtensions::Collections::LazyCollectorStep#to_str gives Middleman::CoreExtensions::Collections::LazyCollectorStep) (TypeError) * bundle aruba 1 that supports Ruby 3.2 * "The use of "#only_processes" is deprecated. Use "#all_commands" instead" * bundle liquid 4 that supports Ruby 3.2 * Support Liquid 4 that doesn't take a context instance for read_template_file Shopify/liquid#441 * the mode of filesystem object should have => should have permission The use of step "the mode of filesystem object "([^"]*)" should (not )?match "([^"]*)" is deprecated. Use "^the (?:file|directory)(?: named)? "([^"]*)" should have permissions "([^"]*)"$" instead') cucumber/aruba@9abe321 * the default aruba timeout is => the default aruba exit timeout is cucumber/aruba@cb9b286 * announcer => aruba.announcer cucumber/aruba@7f8a155 * `@interactive` => last_command_started cucumber/aruba@9373642 * aruba.announcer.stdout => aruba.announcer.announce :stdout cucumber/aruba@9392ece * TimeoutError => Timeout::Error for Ruby 3.1+ ruby/timeout@f9a9758a41 * exit_timeout => aruba.config.exit_timeout cucumber/aruba@cad2b1b * unescape => unescape_text cucumber/aruba@20bffb7 * run => run_command cucumber/aruba@4b94acc * current_dir => aruba.current_directory cucumber/aruba@12643d6 * Pass `exit_timeout` option into `run_command` as a kwarg cucumber/aruba@3c46eec * set_env => set_environment_variable cucumber/aruba@663991e
We want to remove context from read_template_file so we can move parsing of partials to the time of parsing the template. This is to allow for better caching of parsed templates in the future.
New method signature for implementing your own liquid filesytem: