Skip to content

Fix warnings around Singleton instance method redefinition #128

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 17, 2015
Merged

Fix warnings around Singleton instance method redefinition #128

merged 1 commit into from
Oct 17, 2015

Conversation

carbonin
Copy link
Member

@carbonin carbonin commented Oct 7, 2015

Calling .init on Singleton directly was redefining the .instance method
for the singleton class.
This change calls the private method #refresh on the FSTab instance rather
than attempting to reinitialize the instance.

@Fryguy
Copy link
Member

Fryguy commented Oct 7, 2015

Should the clearing of the singleton cache be done higher as it was previously but just in a different way?

@carbonin
Copy link
Member Author

carbonin commented Oct 7, 2015

The way it was done before didn't actually call the #refresh method so it didn't need to be stubbed out. If I added it as a before I would need to stub there also and because I might be using instance for the first time I would need to stub multiple calls which could eat up the calls to File.read we are expecting in the tests.

bdunne
bdunne reviewed Oct 15, 2015
View reviewed changes
spec/fstab_spec.rb Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only be read once, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was to account for if it was the first time we referenced instance and also when we call refresh

@carbonin carbonin force-pushed the remove_singleton_method_definition_warnings branch from 3dedf26 to aa7d7b6 Compare October 16, 2015 20:38
@carbonin
Fix warnings around Singleton instance method redefinition
e06050d 
Calling .__init__ on Singleton directly was redefining the .instance method
for the singleton class.
This change uses a new class duplicate for each test rather
than attempting to reinitialize the instance manually.
@carbonin carbonin force-pushed the remove_singleton_method_definition_warnings branch from aa7d7b6 to e06050d Compare October 16, 2015 20:58
@miq-bot
Copy link
Member

miq-bot commented Oct 16, 2015

Checked commit carbonin@e06050d with ruby 1.9.3, rubocop 0.34.2, and haml-lint 0.13.0
1 file checked, 0 offenses detected
Everything looks good. 🍰

bdunne added a commit that referenced this pull request Oct 17, 2015
@bdunne
Merge pull request #128 from carbonin/remove_singleton_method_definit…
d7912c9 
…ion_warnings

Fix warnings around Singleton instance method redefinition
@bdunne bdunne merged commit d7912c9 into ManageIQ:master Oct 17, 2015
@bdunne
Copy link
Member

bdunne commented Oct 17, 2015

👍

@carbonin carbonin deleted the remove_singleton_method_definition_warnings branch November 13, 2015 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@carbonin @Fryguy @miq-bot @bdunne