Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer requirement of securerandom, use regular require #511

Merged
merged 2 commits into from
Jan 14, 2020
Merged

Defer requirement of securerandom, use regular require #511

merged 2 commits into from
Jan 14, 2020

Conversation

djberg96
Copy link
Contributor

Since #218 we've been seeing this warning in the console and/or rails logs:

securerandom.rb:275: warning: already initialized constant Random::Formatter::ALPHANUMERIC
securerandom.rb:275: warning: previous definition of ALPHANUMERIC was here

This appears to be caused by load-ing securerandom after it's already been require'd somewhere else.

I'm not sure why it was written this way since securerandom should already be loaded (via rails, in active_support.rb, among other places). Since it's only used within a single method, I've moved it into that method so that it's deferred until needed, and changed it to a regular require. The specs still seem to pass.

@djberg96
Copy link
Contributor Author

@miq-bot add_label refactoring

@miq-bot
Copy link
Member

miq-bot commented Jan 14, 2020

Checked commits https://github.com/djberg96/manageiq-providers-vmware/compare/a6b665e93628e808dcebdd8d82d5b3ab54d430e6~...04ce352a911d8d3797969a713645bf226e5f14d8 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@Fryguy Fryguy merged commit 127f86a into ManageIQ:master Jan 14, 2020
@Fryguy Fryguy added this to the Sprint 128 Ending Jan 20, 2020 milestone Jan 14, 2020
@Fryguy Fryguy self-assigned this Jan 14, 2020
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