Add erblint #1646
Add erblint #1646
Conversation
murny
requested review from
ConnorSheremeta,
lagoan,
mbarnett and
pgwillia
as code owners
Generated by 🚫 Danger |
.erb-lint.yml
Outdated
| exclude: | ||
| - '**/*.en.html.erb' | ||
|
|
||
| # ErbSafety: |
There was a problem hiding this comment.
This basically runs https://github.com/Shopify/better-html. So it be nice to have this. Currently with default config its complaining about our usages of raw or .html_safe in a few places (like kaminari pagination views). So might provide our own config to disable these offenses so we continue to have this turned on.
There was a problem hiding this comment.
Added a code comment here and left this turned off. We will leave this as disabled for time being. I tried to add an .better-html.yml config, but doesn't seem to be as fleshed out as erblint or rubocop configs. Doesn't seem like you can exclude cops, only change their values. Very little documentation about this as well. Better-html brings in only ~5 cops and we will probably turn off 2 of them, so not seeing if this is worth the hassle. Will leave it off. Maybe in future better documentation will be there (this is probably the reason why this is disabled as a default as well).
| @@ -60,6 +60,9 @@ jobs: | |||
| DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_GITHUB_API_TOKEN }} | |||
| run: bundle exec danger | |||
|
|
|||
| - name: Run ERBLint | |||
There was a problem hiding this comment.
Need to look into this. Currently failing with lint failures that I don't see on local. Looks like its validating down into our Gems which is causing a bunch of false positives. Will fix this
| @@ -2,7 +2,9 @@ | |||
| <!-- Google Analytics --> | |||
| <script async src="https://www.googletagmanager.com/gtag/js?id=<%= Rails.application.secrets.google_analytics_token %>"></script> | |||
| <%# TODO: javascript_tag, nonce: true do %> | |||
| <%= javascript_tag do %> | |||
There was a problem hiding this comment.
They really don't like javascript_tag. I really couldn't find out why either? From looking into their source It seems like they are not able to valid ERB inside them so they prefer you don't have them in your project. They autocorrect it to just replace it with what it would of rendered anyways (a <script> tag)
| rubocop_config: | ||
| inherit_from: | ||
| - .rubocop.yml | ||
| Layout/InitialIndentation: |
There was a problem hiding this comment.
Using their recommendation of turning off these cops. We could maybe look into this more
| @@ -195,6 +195,8 @@ en: | |||
| results: | |||
| no_items_found: 'No items found' | |||
| skip_to_results: 'Skip to Search Results' | |||
| range_facet_result: | |||
| to: 'to' | |||
There was a problem hiding this comment.
Nice, it even caught some hard-coded English we missed.
There was a problem hiding this comment.
yeah there was 5 places we hardcoded stuff. Which is pretty good with how many views we got!
#989 Brings in ERBLint for Jupiter
Majority of the fixes are white space diffs (will document when that is not the case)
I also added this to CI, but seems to be breaking (looks like its linting our Gems which we should be ignoring 🤔 )