@pablobm commented on this pull request.
> + Array.wrap(Settings.linkify&.detection_rules)
+ .select { |rule| rule.path_template && rule.patterns.is_a?(Array) }
+ .flat_map { |rule| linkify_detection_rule_to_gsub(rule) }
The issue is with the state it creates (the class variable). It lingers on,
sometimes in places where it's not expected.
A couple of examples that come to mind:
- State remains across tests, introducing flaky failures if not careful.
- Values generated in a non-threadsafe manner, causing unexpected behaviour.
In this case I think the tests are affected, as they set things up with
`with_settings` and the memoization will not be reset across tests. In the past
I have worked around that with an explicit `RichText.reset_linkify_settings!`
(for example) called on `setup` of affected tests, but it adds complication.
As for the thread safety, building the list in an initializer might be enough.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6518#discussion_r2589868309
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/6518/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
