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

rm duplicate activated.tag(s) #153

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

looseyi
Copy link

@looseyi looseyi commented Feb 16, 2021

cause new PossibilityState already push the state.

state.activated.tag(state)

cause new PossibilityState already push the state.
```
state.activated.tag(state)
```
@dnkoutso
Copy link
Contributor

dnkoutso commented Aug 4, 2021

any tests to add here or do existing ones cover this?

@dnkoutso
Copy link
Contributor

dnkoutso commented Aug 9, 2021

Is there a test case we can add here or do existing ones cover this? Or was it just not used?

Should we also add a CHANGELOG entry?

@looseyi
Copy link
Author

looseyi commented Aug 10, 2021 via email

Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Makes sense to me, although not sure how to cover this with a fix. It's just duplicated work already done by state.pop_possibility_state. Also, I think state.pop_possibility_state always instantiates a new possibility state, so the if is unnecessary too?

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.

None yet

4 participants