/growstuff

Removed redundant returns #1116

Merged
merged 4 commits into from Dec 6, 2016

Reviewers

@CloCkWeRX CloCkWeRX

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

  Release 17

5 participants

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

No description provided.

@Br3nda Br3nda Removed redundant returns

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
92195d5
@maco maco added the in progress label Dec 4, 2016
@Br3nda Br3nda Line no longer needs wrapping

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
26e5a41
app/helpers/harvests_helper.rb
@@ -19,20 +19,16 @@ def display_human_quantity(harvest)
if harvest.unit == 'individual' # just the number
number_to_human(harvest.quantity, strip_insignificant_zeros: true)
elsif !harvest.unit.blank? # pluralize anything else
- return pluralize(number_to_human(harvest.quantity, strip_insignificant_zeros: true), harvest.unit)
+ pluralize(number_to_human(harvest.quantity, strip_insignificant_zeros: true), harvest.unit)
@CloCkWeRX
CloCkWeRX Dec 5, 2016 Contributor

I'd be kind of inclined to refactor this method to return early, so theres less nested conditionals
return unless !harvest.quantity.blank? && harvest.quantity > 0

@Br3nda
Br3nda Dec 5, 2016 Contributor

That looks much prettier

app/helpers/harvests_helper.rb
end
- else
- return nil
end
end
def display_weight(harvest)
if !harvest.weight_quantity.blank? && harvest.weight_quantity > 0
@CloCkWeRX
CloCkWeRX Dec 5, 2016 Contributor

Ditto

app/models/seed.rb
@@ -66,17 +66,17 @@ class Seed < ActiveRecord::Base
def tradable?
if self.tradable_to == 'nowhere'
@CloCkWeRX
CloCkWeRX Dec 5, 2016 Contributor

Can be a one liner instead
self.tradable_to != 'nowhere'

@CloCkWeRX
Contributor
CloCkWeRX commented Dec 5, 2016

Mmm, those tests are being odd. I can't see how https://travis-ci.org/Growstuff/growstuff/jobs/181238816 is failing because of 5517139 alone.
@Br3nda
Contributor
Br3nda commented Dec 5, 2016 • edited

I'm running some git bisect on this
Br3nda added some commits Dec 5, 2016
@Br3nda Br3nda Simplifying display_harvest_description

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
91b0c18
@Br3nda Br3nda Simplified display_weight

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
8981a22
@coveralls
coveralls commented Dec 6, 2016 • edited

Coverage Status

Coverage decreased (-0.005%) to 87.754% when pulling 8981a22 on Br3nda:bw/returns into 0fcd8c8 on Growstuff:dev.
@CloCkWeRX CloCkWeRX merged commit 41e408b into Growstuff:dev Dec 6, 2016
2 checks passed

2 checks passed

Details codeclimate Code Climate didn't find any new or fixed issues.
Details continuous-integration/travis-ci/pr The Travis CI build passed
@CloCkWeRX CloCkWeRX removed the in progress label Dec 6, 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