@pablobm commented on this pull request.
> + when /^(?>[a-z:_-]+:)?wikipedia$/
+ lang = "en" if key == "wikipedia"
+ when /^(?>[a-z:_-]+:)?wikipedia:([a-z-]{2,12})$/
lang = Regexp.last_match(1)
Perhaps more readable as below?
```suggestion
when /^wikipedia$/
lang = "en"
when /^([a-z_-]+:)?wikipedia$/
lang = nil
when /^([a-z_-]+:)?wikipedia:([a-z-]{2,12})$/
lang = Regexp.last_match(2)
```
I had to look up what `?>` means (never easy to look up!). If it's necessary,
it should be covered by a test for clarity to future readers.
Also I removed the `:` from the brackets. I think that's why the `?>` was
there? Not sure. But again there are no tests showing when it would be useful.
The `[a-z-]{2,12}` is meaningful, used in two places, and the second instance
has a comment explaining it. It should be in a descriptive constant/method.
Similar with `[a-z:_-]+:`.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/7136#pullrequestreview-4475707632
You are receiving this because you are subscribed to this thread.
Message ID:
<openstreetmap/openstreetmap-website/pull/7136/review/[email protected]>_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev
