Skip to content

Conversation

santib
Copy link
Contributor

@santib santib commented Aug 22, 2019

Summary

I first wanted to fix this condition !attachables.nil? || Array(attachables).any? since if something is not nil it will always return true instead of checking if the array has any element. My guess is that this other condition that already existed attachables.nil? || Array(attachables).none? was tried to be negated unsuccessfully.

Then I realised that Array(nil) returns [] so we could simplify the conditions.

Notes

Not sure why tests are failing

@georgeclaghorn
Copy link
Contributor

The original condition (attachables.nil? || Array(attachables).none?) was intended to avoid needless array allocations, but I think it’s clearer this way.

@georgeclaghorn georgeclaghorn merged commit e5c143c into rails:master Aug 22, 2019
@santib santib deleted the fix-activestorage-conditions branch August 22, 2019 20:29
@georgeclaghorn georgeclaghorn added this to the 6.0.1 milestone Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants