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

perf stuff #366

Closed
wants to merge 2 commits into from
Closed

perf stuff #366

wants to merge 2 commits into from

Conversation

jebeck
Copy link
Contributor

@jebeck jebeck commented Dec 2, 2016

I did it @krystophv!!! 🎉 🎊 🎉

We just weren't using webpack right /o\

This PR also contains the addition of the React Perf addon in dev (attached to window so you can use in the console, as described e.g., here: http://benchling.engineering/performance-engineering-with-react/). Sticking it in the redux dev mode <Root /> component seemed like the easiest way (to me) to keep it out of prod, but LMK if you can think of a cleaner way.

@hntrdglss for context: @krystophv noticed a while ago that we were clearly still using the dev build of React in production because propTypes validation shows up all over any perf profiles...

Copy link
Member

@krystophv krystophv left a comment

Choose a reason for hiding this comment

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

This LGTM. I think including the Perf addons in the dev Root will probably be fine. Especially since it's dev-only tooling.

@jebeck
Copy link
Contributor Author

jebeck commented Dec 5, 2016

OK, so let's plan to merge this along with something else upcoming so that the production build gets a smoke test along with whatever main feature is being tested. Either with the smbg Trends port or the CGM trends range selections I guess. Thoughts @krystophv?

@jebeck
Copy link
Contributor Author

jebeck commented Dec 12, 2016

Closing to re-open against @krystophv CGM checkboxes work (#363).

@jebeck jebeck closed this Dec 12, 2016
@jebeck jebeck deleted the jebeck/perf-stuff branch February 14, 2017 21:16
yvon-dblg referenced this pull request in mdblp/blip Dec 1, 2020
Carbs not visualized for calculator bolus that includes carbs but no insulin delivery
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