Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[#91] Add nested interpolation cop #158

Merged
merged 3 commits into from
Feb 13, 2020
Merged

[#91] Add nested interpolation cop #158

merged 3 commits into from
Feb 13, 2020

Conversation

roman-dubrovsky
Copy link
Contributor

@roman-dubrovsky roman-dubrovsky commented Feb 5, 2020 •
edited
Loading

Resolves issue #91 . Also, I'm going to send this cop to the rubocop repo.

Before you submit a pull request, please make sure you have to follow:

  • read and know items from the Contributing Guide
  • add a description of the problem you're trying to solve (short summary from related issue)
  • verified that cops are ordered by alphabet
  • add a note to the style guide docs (if it needs)
  • add a note to the changelog file
  • the commit message contains the number of the related issue (if it presents)
    and word Fix if this PR closes related issue
  • squash all commits before submitting to review

roman-dubrovsky
roman-dubrovsky commented Feb 6, 2020
View reviewed changes
spec/datarockets/style/cop/style/nested_interpolation_spec.rb Outdated
end

it 'accepts interpolation into business block' do
expect_no_offenses('"#{assignment.join("\n#{indentation(node)}")}"')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you think should we ban that construction? Or we can accept it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roman-dubrovsky , I think it is redundant and should be banned.

@roman-dubrovsky roman-dubrovsky added blocked help wanted Extra attention is needed labels Feb 6, 2020
@roman-dubrovsky roman-dubrovsky force-pushed the feature/style-double-interpolation branch from 45c7cb7 to 9a8e4e2 Compare February 6, 2020 13:24
@roman-dubrovsky
Copy link
Contributor Author

@lazycoder9 heh, tests are red 😄

@lazycoder9
Copy link
Contributor

@lazycoder9 heh, tests are red 😄

Yeah, noted it after approve :(

@roman-dubrovsky roman-dubrovsky force-pushed the feature/style-double-interpolation branch from 4187bcd to 2c5e94e Compare February 13, 2020 12:44
@roman-dubrovsky roman-dubrovsky removed blocked help wanted Extra attention is needed labels Feb 13, 2020
@roman-dubrovsky roman-dubrovsky merged commit 8fc7f04 into master Feb 13, 2020
@roman-dubrovsky roman-dubrovsky deleted the feature/style-double-interpolation branch February 13, 2020 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@texpert texpert texpert left review comments

@lazycoder9 lazycoder9 lazycoder9 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@roman-dubrovsky @lazycoder9 @texpert