From: Tom Hughes Date: Tue, 26 Mar 2019 21:11:51 +0000 (+0000) Subject: Merge remote-tracking branch 'upstream/pull/2168' X-Git-Tag: live~2671 X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/c82fdf8e0fd4e5a8f2d831592d5e1fe13e1fe558?hp=dcfe326f6515cc7aeb9c02b7536e69863c60cd01 Merge remote-tracking branch 'upstream/pull/2168' --- diff --git a/.erb-lint.yml b/.erb-lint.yml new file mode 100644 index 000000000..29057feb2 --- /dev/null +++ b/.erb-lint.yml @@ -0,0 +1,57 @@ +--- +linters: + SelfClosingTag: + enabled: false + SpaceInHtmlTag: + enabled: false # TODO + Rubocop: + enabled: true + rubocop_config: + inherit_from: + - .rubocop.yml + Layout/InitialIndentation: + Enabled: false + Layout/TrailingBlankLines: + Enabled: false + Layout/TrailingWhitespace: + Enabled: false + Naming/FileName: + Enabled: false + Style/FrozenStringLiteralComment: + Enabled: false + Metrics/LineLength: + Enabled: false + Lint/UselessAssignment: + Enabled: false + Rails/OutputSafety: + Enabled: false + Style/StringLiterals: + Enabled: false # TODO + Style/BracesAroundHashParameters: + Enabled: false # TODO + Rails/DynamicFindBy: + Enabled: false # TODO + Style/AndOr: + Enabled: false # TODO + Style/WordArray: + Enabled: false # TODO + Style/MethodCallWithoutArgsParentheses: + Enabled: false # TODO + Layout/LeadingBlankLines: + Enabled: false # TODO + Style/NestedParenthesizedCalls: + Enabled: false # TODO + Rails/LinkToBlank: + Enabled: false # TODO + Style/HashSyntax: + Enabled: false # TODO + Style/SymbolProc: + Enabled: false # TODO + Style/Not: + Enabled: false # TODO + Style/NegatedIf: + Enabled: false # TODO + Style/ConditionalAssignment: + Enabled: false # TODO +exclude: + - '**/vendor/**' diff --git a/.travis.yml b/.travis.yml index 1b99a5143..9b848200f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,4 +28,5 @@ before_script: script: - bundle exec rubocop -f fuubar - bundle exec rake jshint + - bundle exec erblint . - bundle exec rake test:db diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 43b74224f..c5a636569 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -3,15 +3,15 @@ ## Coding style -When writing code it is generally a good idea to try and match your -formatting to that of any existing code in the same file, or to other -similar files if you are writing new code. Consistency of layout is -far more important than the layout itself as it makes reading code -much easier. - -One golden rule of formatting -- please don't use tabs in your code -as they will cause the file to be formatted differently for different -people depending on how they have their editor configured. +We use [Rubocop](https://github.com/rubocop-hq/rubocop) (for ruby files) +and [ERB Lint](https://github.com/Shopify/erb-lint) (for erb templates) +to help maintain consistency in our code. You can run these utilities during +development to check that your code matches our guidelines: + +``` +bundle exec rubocop +bundle exec erblint . +``` ## Testing diff --git a/Gemfile b/Gemfile index 83f298101..b0b5f4950 100644 --- a/Gemfile +++ b/Gemfile @@ -144,6 +144,7 @@ end group :development, :test do gem "capybara", "~> 2.13" gem "coveralls", :require => false + gem "erb_lint", :require => false gem "factory_bot_rails" gem "jshint" gem "poltergeist" diff --git a/Gemfile.lock b/Gemfile.lock index 57feba1fd..e52ed910e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -64,6 +64,14 @@ GEM coderay (>= 1.0.0) erubi (>= 1.0.0) rack (>= 0.9.0) + better_html (1.0.11) + actionview (>= 4.0) + activesupport (>= 4.0) + ast (~> 2.0) + erubi (~> 1.4) + html_tokenizer (~> 0.0.6) + parser (>= 2.4) + smart_properties bigdecimal (1.1.0) binding_of_caller (0.8.0) debug_inspector (>= 0.0.1) @@ -145,6 +153,13 @@ GEM dry-logic (~> 0.5, >= 0.5.0) dry-types (~> 0.14, >= 0.14) dynamic_form (1.1.4) + erb_lint (0.0.28) + activesupport + better_html (~> 1.0.7) + html_tokenizer + rainbow + rubocop (~> 0.51) + smart_properties erubi (1.8.0) execjs (2.7.0) exifr (1.3.6) @@ -165,6 +180,7 @@ GEM activesupport (>= 4.2.0) hashdiff (0.3.8) hashie (3.6.0) + html_tokenizer (0.0.7) htmlentities (4.3.4) http_accept_language (2.0.5) i18n (0.9.5) @@ -381,6 +397,7 @@ GEM json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) + smart_properties (1.13.1) sprockets (3.7.2) concurrent-ruby (~> 1.0) rack (> 1, < 3) @@ -443,6 +460,7 @@ DEPENDENCIES dalli delayed_job_active_record dynamic_form + erb_lint factory_bot_rails fakefs faraday diff --git a/app/views/browse/_common_details.html.erb b/app/views/browse/_common_details.html.erb index 17a5f1f40..3b3030abf 100644 --- a/app/views/browse/_common_details.html.erb +++ b/app/views/browse/_common_details.html.erb @@ -26,7 +26,7 @@ <% if @type == "node" and common_details.visible? %>
<%= t 'browse.location' %> - <%= link_to(content_tag(:span, number_with_delimiter(common_details.lat), :class => "latitude") + ", " + content_tag(:span, number_with_delimiter(common_details.lon), :class => "longitude"), {:controller => 'site', :action => 'index', :anchor => "map=18/#{common_details.lat}/#{common_details.lon}"}) %> + <%= link_to(content_tag(:span, number_with_delimiter(common_details.lat), :class => "latitude") + ", " + content_tag(:span, number_with_delimiter(common_details.lon), :class => "longitude"), { :controller => 'site', :action => 'index', :anchor => "map=18/#{common_details.lat}/#{common_details.lon}" }) %>
<% end %> diff --git a/app/views/browse/_containing_relation.html.erb b/app/views/browse/_containing_relation.html.erb index 1b26a89b6..391462d92 100644 --- a/app/views/browse/_containing_relation.html.erb +++ b/app/views/browse/_containing_relation.html.erb @@ -5,4 +5,5 @@ else raw t '.entry_role', :relation_name => linked_name, :relation_role => h(containing_relation.member_role) end -%> + %> + diff --git a/app/views/browse/_relation.html.erb b/app/views/browse/_relation.html.erb index e187da6ca..e7c4817b0 100644 --- a/app/views/browse/_relation.html.erb +++ b/app/views/browse/_relation.html.erb @@ -11,7 +11,7 @@ <%= render :partial => "common_details", :object => relation %> <% unless relation.containing_relation_members.empty? %> -

<%= t'browse.part_of' %>

+

<%= t 'browse.part_of' %>

<% end %> diff --git a/app/views/browse/_relation_member.html.erb b/app/views/browse/_relation_member.html.erb index cedc6f5e9..5b6b5c365 100644 --- a/app/views/browse/_relation_member.html.erb +++ b/app/views/browse/_relation_member.html.erb @@ -3,10 +3,12 @@ linked_name = link_to printable_name(relation_member.member), { :action => relation_member.member_type.downcase, :id => relation_member.member_id.to_s }, :title => link_title(relation_member.member), :rel => link_follow(relation_member.member) type_str = t '.type.' + relation_member.member_type.downcase %> -
  • <%= +
  • + <%= if relation_member.member_role.blank? raw t '.entry', :type => type_str, :name => linked_name else raw t '.entry_role', :type => type_str, :name => linked_name, :role => h(relation_member.member_role) end - %>
    • + %> + diff --git a/app/views/browse/_way.html.erb b/app/views/browse/_way.html.erb index 4f331b5e9..7099f015a 100644 --- a/app/views/browse/_way.html.erb +++ b/app/views/browse/_way.html.erb @@ -11,7 +11,7 @@ <%= render :partial => "common_details", :object => way %> <% unless way.containing_relation_members.empty? %> -

    <%= t'browse.part_of' %>

    +

    <%= t 'browse.part_of' %>

    @@ -25,7 +25,7 @@ <%= link_to printable_name(wn.node), { :action => "node", :id => wn.node_id.to_s }, :class => link_class('node', wn.node), :title => link_title(wn.node), :rel => link_follow(wn.node) %> <% related_ways = wn.node.ways.reject { |w| w.id == wn.way_id } %> <% if related_ways.size > 0 then %> - (<%= raw t '.also_part_of', :count => related_ways.size, :related_ways => related_ways.map { |w| link_to(printable_name(w), { :action => "way", :id => w.id.to_s }, :class => link_class('way', w), :title => link_title(w) ) }.to_sentence %>) + (<%= raw t '.also_part_of', :count => related_ways.size, :related_ways => related_ways.map { |w| link_to(printable_name(w), { :action => "way", :id => w.id.to_s }, :class => link_class('way', w), :title => link_title(w)) }.to_sentence %>) <% end %> <% end %> diff --git a/app/views/browse/changeset.html.erb b/app/views/browse/changeset.html.erb index 1328461d7..1d09cbedf 100644 --- a/app/views/browse/changeset.html.erb +++ b/app/views/browse/changeset.html.erb @@ -36,8 +36,9 @@
  • <%= t(".commented_by", - :when => friendly_date(comment.created_at), :exact_time => l(comment.created_at), - :user => link_to(h(comment.author.display_name), user_path(comment.author))).html_safe %> + :when => friendly_date(comment.created_at), + :exact_time => l(comment.created_at), + :user => link_to(h(comment.author.display_name), user_path(comment.author))).html_safe %> <% if current_user and current_user.moderator? %> — <%= t('javascripts.changesets.show.hide_comment') %> <% end %> @@ -48,8 +49,9 @@
  • <%= t(".hidden_commented_by", - :when => friendly_date(comment.created_at), :exact_time => l(comment.created_at), - :user => link_to(h(comment.author.display_name), user_path(comment.author))).html_safe %> + :when => friendly_date(comment.created_at), + :exact_time => l(comment.created_at), + :user => link_to(h(comment.author.display_name), user_path(comment.author))).html_safe %> — <%= t('javascripts.changesets.show.unhide_comment') %> <%= comment.body.to_html %> @@ -109,7 +111,7 @@ <% unless @nodes.empty? %>

    <%= type_and_paginated_count('node', @node_pages) %> - <%= render :partial => 'paging_nav', :locals => { :pages => @node_pages, :page_param => "node_page"} %> + <%= render :partial => 'paging_nav', :locals => { :pages => @node_pages, :page_param => "node_page" } %>

    - <%= render :partial => "contact", :collection => friends, :locals => {:type => "friend"} %> + <%= render :partial => "contact", :collection => friends, :locals => { :type => "friend" } %>
    <% end %> @@ -226,7 +226,7 @@
  • <%= link_to t('.nearby_diaries'), nearby_diaries_path %>
    • - <%= render :partial => "contact", :collection => nearby, :locals => {:type => "nearby mapper"} %> + <%= render :partial => "contact", :collection => nearby, :locals => { :type => "nearby mapper" } %>
    <% end %> diff --git a/app/views/users/terms.html.erb b/app/views/users/terms.html.erb index a836c8590..acd617653 100644 --- a/app/views/users/terms.html.erb +++ b/app/views/users/terms.html.erb @@ -7,7 +7,7 @@
    <% end %> -<%= form_tag({:action => "save"}, { :class => " inner22 standard-form fillL" }) do %> +<%= form_tag({ :action => "save" }, { :class => " inner22 standard-form fillL" }) do %>
    - (<%= link_to(t('.consider_pd_why'), t('.consider_pd_why_url'), :target => :new)%>) + (<%= link_to(t('.consider_pd_why'), t('.consider_pd_why_url'), :target => :new) %>) <%= hidden_field_tag('referer', h(params[:referer])) unless params[:referer].nil? %>