Optimize default front matter using File.fnmatch?
#9185
Conversation
test/test_front_matter_defaults.rb
Outdated
| end | ||
|
|
||
| should "affect the appropriate items only" do | ||
| item = @staff.docs.sample |
There was a problem hiding this comment.
Array#sample selects a random item, so if you're expecting to always have the same item come out, then use #shift or #[].
There was a problem hiding this comment.
Actually, the intention here is to indeed test a random object in the homogeneous array during each run.
There was a problem hiding this comment.
Every doc is the same, or the array is of size 1? I’m still confused about this. If it’s one element, it would be clearer to first assert the size, then assert its contents. If the array has >1 element that you expect to be the same, wouldn’t it be more effective to iterate over all elements and assert each element has the contents you want.
what I’m afraid of is that we’ll add a new doc or file to this collection in the fixture site and these tests will randomly fail rather than consistently fail. I think tests should be deterministic to avoid contributors chasing down confusing errors that only occur sometimes.
test/test_front_matter_defaults.rb
Outdated
|
|
||
| should "call Dir.glob block" do | ||
| assert_includes @output, "Globbed Scope Path:" | ||
| item = @staff.files.sample |
|
Interesting Ruby 3.1 failure... couldn't compile C extensions for liquid-c. |
|
Restarted it to see if it was a one-off failure.. |
|
This could be because of recent release Ruby 3.1.3 or maybe not. |
|
@parkr Can I backport this change to |
I took a look at the repo for issues and noticed that there's a 4.1.0 release but the error looks like a 4.0.0 release. Are we able to use the latest 4.x release of liquid-c? |
I guess, but why? Is 4.4 a big upgrade from 4.3 such that folks couldn't upgrade easily? We don't need to provide every optimization to every minor branch, but if you think folks are prevented from upgrading then go for it. |
|
@jekyllbot: merge +fix |
Ashwin Maroli: Optimize default front matter using `File.fnmatch?` (#9185) Merge pull request 9185
Summary
Instead of using
Dir.globto create a list of matching paths and testing given relative_patch against each list entry, useFile.fnmatch?to directly test if given relative path matches configuredscope["path"]with glob pattern sigil (*).Notes
scope["path"]. But for backwards-compatibility, continue acceptingscope["path"]with collections_dir and strip it away under the hood. TODO: Deprecate this need in a subsequent pull request.