Skip to content

Extract IssetHelper from MutatingScope #1783

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

Closed
wants to merge 1 commit into from

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Oct 2, 2022

This would be the precondition to cleanup IssetCheck and to another PR I'm currently working on :)

Not sure about class and method names, any suggestions appreciated

@herndlm herndlm changed the base branch from 1.9.x to 1.8.x October 2, 2022 18:18
@herndlm herndlm force-pushed the extract-isset-helper branch 3 times, most recently from 09c57e7 to 8cb2861 Compare October 2, 2022 18:34
@herndlm
Copy link
Contributor Author

herndlm commented Oct 2, 2022

weird, I don't understand what's going on with the incompatible ScopeFactory::create error, I can't explain how the changes here would lead to that. Is this related to downgrading the code?

@herndlm herndlm force-pushed the extract-isset-helper branch 3 times, most recently from 5022643 to c675c4a Compare October 2, 2022 20:00
@herndlm herndlm force-pushed the extract-isset-helper branch from c675c4a to 7946a35 Compare October 2, 2022 20:02
@herndlm
Copy link
Contributor Author

herndlm commented Oct 2, 2022

also: I just realised now that IssetCheck is not a rule but already a helper class for a rule.. 😅

@herndlm herndlm marked this pull request as draft October 2, 2022 21:05
@herndlm
Copy link
Contributor Author

herndlm commented Oct 2, 2022

I guess the goal could be to use either MutatingScope::issetCheck or the IssetCheck class. No need for another way of doing this :) I'll try to figure it out

@herndlm herndlm closed this Oct 2, 2022
@herndlm herndlm deleted the extract-isset-helper branch October 3, 2022 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant