ManageIQ / more_core_extensions

MoreCoreExtensions are a set of core extensions beyond those provided by ActiveSupport.
MIT License
5 stars 23 forks source link

Extend the Digest::UUID module with the clean method #81

Closed djberg96 closed 4 years ago

djberg96 commented 4 years ago

Since Rails 5.1+ gives us the Digest::UUID class, let's extend it with the clean method here. The eventual goal is to update the core Hardware class with this method, and remove miq-uuid from gems-pending.

djberg96 commented 4 years ago

We can ignore the Hakiri warning, it's a bug on their end.

bdunne commented 4 years ago

I know you're just moving code from a different repo, but when I put on my non-ManageIQ more_core_extensions hat :tophat: I wonder if this should instead be one or more of:

@Fryguy Any thoughts?

Fryguy commented 4 years ago

I don't like the string patches, because to_uuid I would expect some kind of UUID object out the other side, not for it to be cleaned. Same with .from_string. .format is not bad, but I think .clean is clearer.

miq-bot commented 4 years ago

Checked commits https://github.com/djberg96/more_core_extensions/compare/3d731e860bbc40a42868f5e65f743b62515b06b3~...d252b90d5dd2e491616215de754a3f9e45eac8f0 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint 4 files checked, 2 offenses detected

lib/more_core_extensions/core_ext/digest/uuid.rb

djberg96 commented 4 years ago

@Fryguy Look ok now?

bdunne commented 4 years ago

@Fryguy Any concerns?