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

Fix memoized method subjects to preserve freezer option #973

Merged
merged 5 commits into from
Aug 24, 2020

Conversation

snusnu
Copy link
Collaborator

@snusnu snusnu commented Oct 28, 2019

Close #639

@snusnu snusnu requested a review from mbj October 28, 2019 12:31
@snusnu snusnu self-assigned this Oct 28, 2019
@snusnu
Copy link
Collaborator Author

snusnu commented Oct 28, 2019

@mbj Ready for review. I see CI is failing integration tests but I'm not sure if you need me to recurse into these.

@mbj
Copy link
Owner

mbj commented Oct 28, 2019

@snusnu Code wise I'm fine, but the CI rusted. Hence it has to be fixed first.

@snusnu snusnu force-pushed the change/preserve-adamantium-freezer-option branch 4 times, most recently from 92310a8 to b194e4c Compare October 29, 2019 02:28
@snusnu snusnu force-pushed the change/preserve-adamantium-freezer-option branch from b194e4c to ad6657b Compare October 29, 2019 02:41
@mbj mbj force-pushed the change/preserve-adamantium-freezer-option branch from 151a7c9 to 18f070d Compare January 1, 2020 01:30
mbj
mbj previously approved these changes Jan 1, 2020
@mbj mbj self-requested a review January 1, 2020 01:39
@snusnu snusnu force-pushed the change/preserve-adamantium-freezer-option branch from 29ca4d4 to 34c3baa Compare February 4, 2020 13:18
@mbj mbj force-pushed the change/preserve-adamantium-freezer-option branch from c430ab8 to a06b0c3 Compare August 24, 2020 14:47
mbj and others added 5 commits August 24, 2020 14:53
* If Memoizable alone is included in the subject scope, #memoize
  does not support any options.
* Adamantium extends Memoizable by adding support for configuring
  the freezer to be used as an option for #memoize.
* If Adamantium is included in the subject scope, the freezer that
  was either used implicitly or configured explicitly, needs to be
  preserved to keep original semantics intact.
* The AST generated for the call to #memoize exhibits semantics
  that are equivalent to the originally defined memoizer. It may
  not always lead to identical source when being unparsed though.
  This is because memoizable provides no API to reflect on whether
  the freezer was configured implicitly by the adamantium module
  or whether it was set explicitly via the :freezer option added
  by adamantium.
@mbj mbj force-pushed the change/preserve-adamantium-freezer-option branch from f1ff55d to 1d2c654 Compare August 24, 2020 14:53
@@ -137,7 +153,7 @@ def foo; end
it_should_behave_like 'a command method'
end

describe '#mutations', mutant_expression: 'Mutant::Subject#mutations' do
describe '#mutations' do
Copy link
Owner

Choose a reason for hiding this comment

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

@snusnu The fix for the last alive mutation was removing this overly specific overwrite!

@mbj mbj merged commit a2bf9df into master Aug 24, 2020
@mbj mbj deleted the change/preserve-adamantium-freezer-option branch August 24, 2020 15:03
@mbj mbj mentioned this pull request Aug 24, 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.

Fix memoizable insertion to preserve freezer option
2 participants