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.

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

Do not raise when variable is defined #921

Merged
merged 4 commits into from
Aug 18, 2017
Merged

Do not raise when variable is defined #921

merged 4 commits into from
Aug 18, 2017

Conversation

pascalbetz
Copy link
Contributor

Variables which are defined but have a nil value should not raise.

See #749

Variables which are defined but have a nil value should not raise.

See #749
@fw42
Copy link
Contributor

fw42 commented Aug 18, 2017

Can you fix the style violations?

@pascalbetz
Copy link
Contributor Author

Fixed the ones that I introduced. The others should be fixed in another PR, I guess.

@fw42
Copy link
Contributor

fw42 commented Aug 18, 2017

ugh sorry, I wasn't aware that things are failing on master too

@fw42
Copy link
Contributor

fw42 commented Aug 18, 2017

Hm it's not failing for me locally. Not sure why it's failing on travis.

.rubocop.yml Outdated
@@ -73,9 +73,6 @@ Style/Documentation:
Style/ClassAndModuleChildren:
Enabled: false

Style/TrailingComma:
Enabled: false
Copy link
Contributor

Choose a reason for hiding this comment

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

The style violations in existing code are because of this change to the rubocop style, which shouldn't be needed for this PR.

@pascalbetz
Copy link
Contributor Author

Hmm, picked up other rubycop with me, fixed.

@pushrax pushrax merged commit a979b3e into Shopify:master Aug 18, 2017
@pascalbetz
Copy link
Contributor Author

@dylanahsmith @fw42 thanks, that was rather fast from PR to acceptance 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants