Apply Rubocop rules in ERB files #4102
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![dependabot-preview[bot]](https://avatars.githubusercontent.com/in/2141?s=60&v=4){kind=small}
dependabot-preview
bot
added
the
dependencies
dependabot-preview
bot
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
b409980
to
ff47934
Compare
dependabot-preview
bot
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
2 times, most recently
from
2bb5d22
to
347541e
Compare
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
4 times, most recently
from
6a666ef
to
b164efa
Compare
houndci-bot
reviewed
Sep 30, 2020
There was a problem hiding this comment.
Some files could not be reviewed due to errors:
.rubocop.yml: Rails/SafeNavigation has the wrong namespace - should be Style
.rubocop.yml: Rails/SafeNavigation has the wrong namespace - should be Style Error: unrecognized cop Performance/CompareWithBlock found in .rubocop.yml, unrecognized cop Performance/DoubleStartEndWith found in .rubocop.yml, unrecognized cop Performance/EndWith found in .rubocop.yml, unrecognized cop Performance/StartWith found in .rubocop.yml, unrecognized cop Rails/Date found in .rubocop.yml, unrecognized cop Rails/EnvironmentComparison found in .rubocop.yml, unrecognized cop Rails/Presence found in .rubocop.yml, unrecognized cop Rails/TimeZone found in .rubocop.yml, unrecognized cop Rails/UnknownEnv found in .rubocop.yml, unrecognized cop Rails/CreateTableWithTimestamps found in .rubocop.yml, unrecognized cop Rails/DynamicFindBy found in .rubocop.yml, unrecognized cop Rails/EnumUniqueness found in .rubocop.yml, unrecognized cop Rails/FindBy found in .rubocop.yml, unrecognized cop Rails/FindEach found in .rubocop.yml, unrecognized cop Rails/HasAndBelongsToMany found in .rubocop.yml, unrecognized cop Rails/HasManyOrHasOneDependent found in .rubocop.yml, unrecognized cop Rails/InverseOf found in .rubocop.yml, unrecognized cop Rails/NotNullColumn found in .rubocop.yml, unrecognized cop Rails/OutputSafety found in .rubocop.yml, unrecognized cop Rails/PluralizationGrammar found in .rubocop.yml, unrecognized cop Rails/RelativeDateConstant found in .rubocop.yml, unrecognized cop Rails/RequestReferer found in .rubocop.yml, unrecognized cop Rails/ReversibleMigration found in .rubocop.yml, unrecognized cop Rails/SaveBang found in .rubocop.yml, unrecognized cop Rails/SkipsModelValidations found in .rubocop.yml, unrecognized cop Rails/Validation found in .rubocop.yml
javierm
changed the title
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
bf8ca87
to
425c855
Compare
javierm
requested changes
Oct 1, 2020
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
425c855
to
0d7f989
Compare
houndci-bot
reviewed
Oct 14, 2020
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
0d7f989
to
22861c9
Compare
|
Since version 0.0.29 is not compatible with Rubocop 0.87+, we're upgrading to verstion 0.0.35. |
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
2 times, most recently
from
5ac3fbb
to
d87db0f
Compare
javierm
changed the base branch from
master
to
dependabot/bundler/rubocop-rails-2.6.0
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
d87db0f
to
1ca10ba
Compare
javierm
force-pushed
the
dependabot/bundler/rubocop-rails-2.6.0
branch
from
34e9f74
to
4c69455
Compare
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
1ca10ba
to
270686a
Compare
javierm
force-pushed
the
dependabot/bundler/rubocop-rails-2.6.0
branch
2 times, most recently
from
c6e0dde
to
4658e18
Compare
Base automatically changed from
dependabot/bundler/rubocop-rails-2.6.0
to
master
October 26, 2020 11:04
|
A newer version of erb_lint exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged. |
Bumps [erb_lint](https://github.com/Shopify/erb-lint) from 0.0.28 to 0.0.35. - [Release notes](https://github.com/Shopify/erb-lint/releases) - [Commits](Shopify/erb-lint@v0.0.28...v0.0.35) Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Just like we do for the rest of the gems.
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
2 times, most recently
from
bca174a
to
5090cbb
Compare
Note this rule does still allow us to add new lines after opening tags; it just makes sure that if we do, we also add it in closing tags. Likewise, if we don't add it in the opening tag, it forces us not to add it in the closing tag either. I don't have a strong preference about either style; in these cases I've chosen the latter because it seemed more common in our code.
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
2 times, most recently
from
652194e
to
ebda95e
Compare
So now we remove duplication between .rubocop.yml and the rubocop rules defined for erblint. Having the rules in two places led to some mistakes, like renaming Layout/Tab to Layout/IndentationStyle in `.rubocop.yml` but forgetting to do so in `.erb-lint.yml`. This also means we can use the EnforcedStyle options for Layout/SpaceInsideHashLiteralBraces in views as well. We couldn't implement this feature earlier because it required Ruby 2.4 and due to incompatibilities between versions of erb_lint and versions of rubocop. There are some rules which do not apply to ERB files and so we're disabling them. * Layout/LineLength, Layout/TrailingEmptyLines and Lint/UselssAssignment generate false positives * Rails/OutputSafety is redundant since its functionality is already covered by ERB Lint ErbSafety linter
Note that in Ruby files this rule allows vertical alignment, but doesn't seem to do the same in ERB. Since we only used vertical alignment in one place, and that place also had an unneeded extra space on every aligned line, I've decided to change the code in that place and follow the rule.
This way we can simplify the code and don't have to rely on `.try` statements which are confusing and so we don't allow them in the `Rails/SafeNavigation` Rubocop rule.
The association `organization.user` returns `nil` when the user is hidden. This was discovered thanks to the `Style/AndOr` rule. We were using `and` and `||` on the same line, which is confusing.
We were doing it manually, but Capybara offers an option which does the exact same thing. This way we also apply the NoJavascriptTagHelper ERB rule, which reported one error in the `disable_animations_in_tests` partial.
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
ebda95e
to
b214205
Compare
javierm
approved these changes
Feb 5, 2021
javierm
approved these changes
Feb 5, 2021
javierm
force-pushed
the
dependabot/bundler/erb_lint-0.0.35
branch
from
a63a13c
to
00656e2
Compare
taitus
approved these changes
Feb 8, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objectives
Gem update details
Bumps erb_lint from 0.0.28 to 0.0.35.
Release notes
Sourced from erb_lint's releases.
Commits
fa0fda3bump version1944354Merge pull request #182 from Shopify/drop-ruby-2.43d6801euse unreleased rubocop-shopify version999a49cdebug CI22b7813shopify cops are not compatible with rubocop 0.89 yetec8a50binherit styleguide from gem instead of from config filec83363cRuby 2.4 is EOL73c9a6eMerge pull request #179 from marcandre/rubocop_v1f3b6b0aUpdate for RuboCop v0.87+87b62d8Avoid redundant recalculationsDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Note: This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit.
You can always request more updates by clicking
Bump nowin your Dependabot dashboard.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)@dependabot use these labelswill set the current labels as the default for future PRs for this repo and language@dependabot use these reviewerswill set the current reviewers as the default for future PRs for this repo and language@dependabot use these assigneeswill set the current assignees as the default for future PRs for this repo and language@dependabot use this milestonewill set the current milestone as the default for future PRs for this repo and language@dependabot badge mewill comment on this PR with code to add a "Dependabot enabled" badge to your readmeAdditionally, you can set the following in your Dependabot dashboard: