Skip to content

Refactor Toolbar Props - See #824 #849

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

Merged
merged 1 commit into from
Mar 1, 2019

Conversation

meiamsome
Copy link
Member

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • is from a uniquely-named feature branch and has been rebased on top of the latest master. (If I was asked to make more changes, I have made sure to rebase onto master then too)
  • is descriptively named and links to an issue number, i.e. Fixes #123

Things to note:

I tried to test everything, but it looks like there is no cases that state.ide.infiniteLoop is ever set in the code - there is a detectInfiniteLoops action creator but it is never used. Looks like this was dropped here: 58d0dba#diff-78f15da1e797d1c04ac443d4af43a757L35

I guess the question is, is that state variable dead code or should that be re-added in?

@meiamsome meiamsome force-pushed the feature/toolbar-prop-refactor branch from e6631ea to c0421db Compare February 25, 2019 23:41
@catarak
Copy link
Member

catarak commented Feb 26, 2019

i think detectInfiniteLoops is dead code—you can go ahead and remove it! the loop-protect library needs to be updated anyway see #698.

@catarak
Copy link
Member

catarak commented Feb 26, 2019

and also, tested this, and it is working for me ✨

@catarak
Copy link
Member

catarak commented Mar 1, 2019

i'm going to merge this, and maybe the dead code removal can be done as part of #698 or another ticket?

@catarak catarak merged commit e37926d into processing:master Mar 1, 2019
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.

2 participants