/growstuff

Fixed parentheses that are interp-ed as grouped expression #1119

Merged
merged 3 commits into from Dec 7, 2016

Reviewers

@maco maco

@CloCkWeRX CloCkWeRX

Assignees

No one assigned

Labels

type: codestyle

Projects

None yet

Milestone

  Release 17

5 participants

@Br3nda @coveralls @maco @CloCkWeRX @cesy
@Br3nda
Contributor
Br3nda commented Dec 6, 2016

No description provided.

@Br3nda Br3nda Fixed parentheses that are interp-ed as grouped expression

All checks have passed

2 successful checks
Details
@maco codeclimate — Code Climate didn't find any new or fixed issues.
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
464c570
@maco maco added the in progress label Dec 6, 2016
@coveralls
coveralls commented Dec 6, 2016

Coverage Status

Coverage remained the same at 87.754% when pulling 464c570 on Br3nda:bw/ParenthesesAsGroupedExpression into 41e408b on Growstuff:dev.
@coveralls
coveralls commented Dec 6, 2016

Coverage Status

Coverage remained the same at 87.754% when pulling 464c570 on Br3nda:bw/ParenthesesAsGroupedExpression into 41e408b on Growstuff:dev.
@maco
maco reviewed Dec 6, 2016 View changes

I see this is mostly removing whitespace before () but there are two removed hashes, so just want to check before merging

app/mailers/notifier.rb
@@ -16,7 +16,7 @@ def notify(notification)
@reply_link = reply_link(@notification)
# Encrypting
- @signed_message = verifier.generate ({ member_id: @notification.recipient.id, type: :send_notification_email })
+ @signed_message = verifier.generate(member_id: @notification.recipient.id, type: :send_notification_email)
@maco
maco Dec 6, 2016 Member

was it intentional to ditch the {}?

@Br3nda
Br3nda Dec 6, 2016 Contributor

So, i need I'm really a python dev who pretends to know ruby.
Rubocop complained about the {} being unnecessary, hinting that ruby treats it as a hash regardless. I took it at it's word.

@Br3nda
Br3nda Dec 6, 2016 Contributor

read up some - and you're right. Good catch. Fixing this now

Br3nda added some commits Dec 6, 2016
@Br3nda Br3nda Call to verify options stays as a hash

All checks have passed

2 successful checks
Details
@maco codeclimate — no new or fixed issues
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
05e7a27
@Br3nda Br3nda Merge branch 'dev' into bw/ParenthesesAsGroupedExpression

All checks have passed

2 successful checks
Details
@maco codeclimate — no new or fixed issues
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
00e240c
@maco
Member
maco commented Dec 6, 2016
@Br3nda
Contributor
Br3nda commented Dec 6, 2016

Could someone with the powers fix up coveralls? I would love to be able to check that CI touched the lines that changed

image
@coveralls
coveralls commented Dec 7, 2016 • edited

Coverage Status

Coverage increased (+0.01%) to 87.765% when pulling 00e240c on Br3nda:bw/ParenthesesAsGroupedExpression into 5347748 on Growstuff:dev.
@CloCkWeRX
Contributor
CloCkWeRX commented Dec 7, 2016

There's something funny going on - its clearly there on dev via git.

I got an email notification just yesterday from [email protected] - Tue, Dec 6, 2016 at 5:22 PM (thats before you authored the PR apparently though)


File Name
 
/notifier.rb

 
Pattern Matched
 
Possible Ruby secret key(s)

That's clearly wrong, but I'm wondering if a similar thing is happening with other apps and they think they are protecting us.
@CloCkWeRX CloCkWeRX merged commit e799233 into Growstuff:dev Dec 7, 2016
2 checks passed

2 checks passed

Details codeclimate no new or fixed issues
Details continuous-integration/travis-ci/pr The Travis CI build passed
@CloCkWeRX CloCkWeRX removed the in progress label Dec 7, 2016
@Br3nda Br3nda deleted the Br3nda:bw/ParenthesesAsGroupedExpression branch Dec 8, 2016
@cesy cesy modified the milestone: Release 17 Dec 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment