@pablobm commented on this pull request.


On test/controllers/api/changesets/uploads_controller_test.rb:

Changes in this file are unrelated to the PR. Please remove them.

> +  def browse_tag_value_cell(tag)
+    options = {
+      :class => "py-1 border-secondary-subtle border-start",
+      :dir => "auto"
+    }
+
+    lang = tag_language(tag[0])
+    options[:lang] = lang if lang
+
+    content_tag :td, format_value(tag[0], tag[1]), **options
+  end
+
+  def tag_language(key)
+    key[/\Aname:([a-z]{2,3}(?:-[A-Za-z0-9]+)*)\z/, 1]
+  end
+

Be it one or two helpers, they should be moved to `BrowseTagsHelper`. It avoids 
having to `include BrowseTagsHelper` at the top, and it's more aligned with 
that helper in terms of what the code does.

This will require moving the tests to the correct test class, as well as 
increasing the `Max` value for `Metrics/ModuleLength` in `.rubocop.yml`, which 
I'm guessing might be the reason why this is was put here?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/6854#pullrequestreview-3992048235
You are receiving this because you are subscribed to this thread.

Message ID: 
<openstreetmap/openstreetmap-website/pull/6854/review/[email protected]>
_______________________________________________
rails-dev mailing list
[email protected]
https://lists.openstreetmap.org/listinfo/rails-dev

Reply via email to