search

glacials / friendly_uuid

Short, stateless URL slugs for UUID-based Rails models
Other
48 stars 3 forks source link

Raise `ActiveRecord::RecordNotFound` for `MyModel.find(nil)` #15

Closed ianbayne closed 2 months ago

ianbayne commented 3 months ago

Closes #14.

ianbayne commented 2 months ago

Love it! As far as I'm concerned adding tests is never outside scope, but if you prefer I'm happy to take over making them pass post-merge if the desired behavior isn't clear. Also happy to talk it through.

Whatever you decide, feel free to mark as ready when you are ready to hand it off!

Thanks! Regarding this comment, I meant that adding these tests in might be considered more within the scope of https://github.com/glacials/friendly_uuid/issues/6 since they cover .find_by instead of .find.

At any rate, I think this PR is ready to be reviewed. Please let me know if there's anything I'm misunderstanding or if there's anything you'd like me to change. Thanks!

ianbayne commented 2 months ago

Love it! As far as I'm concerned adding tests is never outside scope, but if you prefer I'm happy to take over making them pass post-merge if the desired behavior isn't clear. Also happy to talk it through. Whatever you decide, feel free to mark as ready when you are ready to hand it off!

Thanks! Regarding this comment, I meant that adding these tests in might be considered more within the scope of #6 since they cover .find_by instead of .find.

At any rate, I think this PR is ready to be reviewed. Please let me know if there's anything I'm misunderstanding or if there's anything you'd like me to change. Thanks!

Just in case you didn't see this, @glacials. No worries if you've been busy!

ianbayne commented 2 months ago

Sorry for the delay, yes this has been in my inbox but I have been busy. The code looks good to me, I'll merge it now and release soon!

No worries at all! Thanks!