Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

35 remove redux logger #36

Merged
merged 6 commits into from
Jan 12, 2018
Merged

Conversation

mjclawar
Copy link
Contributor

@mjclawar mjclawar commented Jan 8, 2018

This PR uses the process.env.NODE_ENV to avoid adding logging to the redux middleware in the production build.
Closes #35

Other changes

  • Adds package-lock.json
  • Adds .idea/ to gitignore for IntelliJ editors
  • Bumps version & adds entry to CHANGELOG
  • Adds small docstring to the initializeStore function.

…all. Update store to not use logger in production build
Add package-lock.json for install.
Update store to not use logger in production build.
Add docstring to initializeStore
@chriddyp
Copy link
Member

chriddyp commented Jan 8, 2018

Looks good to me, thanks @mjclawar !

It looks like tests aren't running, I suspect because this is a forked branch. I'll update those settings.

@chriddyp
Copy link
Member

chriddyp commented Jan 8, 2018

OK @mjclawar , I believe that forks should now trigger tests. Could you try pushing something up again?

@mjclawar
Copy link
Contributor Author

mjclawar commented Jan 8, 2018

@chriddyp Tests all pass on my local machine -- any ideas why that one failed?

@chriddyp
Copy link
Member

chriddyp commented Jan 8, 2018

Hm, I suspect that it's intermittent. I'll re-run the tests

@chriddyp
Copy link
Member

chriddyp commented Jan 8, 2018

        input1.send_keys('hello world')

is supposed to fire the callback len('hello world') times: one for each character. However, #22 introduced some behaviour. From the changelog: "This fix should also improve performance when many updates happen at once as outdated requests will get dropped instead of updating the UI.". In this case, I suspect that some requests are getting processed out of order, requests are getting dropped, and the callback is fired 11 times instead of 12.

We should probably update the test with something like:

for char in 'hello world':
    input_1.send_keys(char)
    time.sleep(0.25)

or something more solid like

text = 'hello world'
for (i, char) in enumerate(text):
    input_1.send_keys(char)
    wait_for_text_to_equal('output', text[:i])

Anyway, we don't need to fix that in this PR. I've logged the issue in #38

@chriddyp chriddyp mentioned this pull request Jan 8, 2018
@mjclawar
Copy link
Contributor Author

@chriddyp do you need anything else from me to merge this PR in and update? Thanks!

@chriddyp
Copy link
Member

@mjclawar Thanks for checking in! No I don't, just need to re-run CI a couple times until everything pases. Circle passes but it looks like Percy had a gateway timeout on the last run.

@chriddyp chriddyp merged commit 767bda7 into plotly:master Jan 12, 2018
@mjclawar
Copy link
Contributor Author

@chriddyp Do you mind publishing 0.11.2 to npm when you get a chance? Thanks! The __init__.py will also need the version bumped to 0.11.2 to work I think.

@chriddyp
Copy link
Member

@mjclawar Thanks for reminder! I just published to npm and pypi

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants