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

[docs] Add error boundary to demos #16871

Merged
merged 1 commit into from
Aug 8, 2019

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Aug 3, 2019

An alternative to #16746 that let our developers fill the issue template, no assistance is provided. It saves us work.

Closes #16209
Closes #16746

Capture d’écran 2019-08-03 à 17 02 07

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Aug 3, 2019
@mui-pr-bot
Copy link

Details of bundle changes.

Comparing: de0c2fd...dccc3ad

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core 0.00% 0.00% 329,716 329,716 90,142 90,142
@material-ui/core/Paper 0.00% 0.00% 68,673 68,673 20,470 20,470
@material-ui/core/Paper.esm 0.00% 0.00% 62,058 62,058 19,209 19,209
@material-ui/core/Popper 0.00% 0.00% 29,185 29,185 10,434 10,434
@material-ui/core/Textarea 0.00% 0.00% 5,759 5,759 2,368 2,368
@material-ui/core/TrapFocus 0.00% 0.00% 3,834 3,834 1,617 1,617
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,389 16,389 5,824 5,824
@material-ui/core/useMediaQuery 0.00% 0.00% 3,221 3,221 1,312 1,312
@material-ui/lab 0.00% 0.00% 152,604 152,604 46,521 46,521
@material-ui/styles 0.00% 0.00% 51,390 51,390 15,286 15,286
@material-ui/system 0.00% 0.00% 15,753 15,753 4,380 4,380
Button 0.00% 0.00% 79,426 79,426 24,280 24,280
Modal 0.00% 0.00% 15,050 15,050 5,233 5,233
Portal 0.00% 0.00% 3,579 3,579 1,568 1,568
Rating 0.00% 0.00% 70,735 70,735 22,079 22,079
Slider 0.00% 0.00% 75,071 75,071 23,272 23,272
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing 0.00% 0.00% 52,044 52,044 13,816 13,816
docs.main +0.17% 🔺 +0.13% 🔺 590,655 591,644 188,787 189,041
packages/material-ui/build/umd/material-ui.production.min.js 0.00% 0.00% 299,810 299,810 86,177 86,177

Generated by 🚫 dangerJS against dccc3ad

@mbrookes
Copy link
Member

mbrookes commented Aug 3, 2019

Given the infrequency of demo errors, this seems like a good tradeoff.

@joshwooding
Copy link
Member

joshwooding commented Aug 6, 2019

I haven’t merged this to allow @eps1lon a chance to see it

@eps1lon
Copy link
Member

eps1lon commented Aug 6, 2019

What do you think of removing the issue template complexity for a direct link to mui-org/material-ui/issues/new/choose? I don't think that it will be used very often.

So let me get this right: You think the user will discard a filled out template with all the information we need and fill this information out themselves? And we shouldn't assist him in anyway because?

From my experience about 10% of the issues are filled to a degree that's workable. I don't share this experience at all most of them are fine as is.

Can you be specific why a filled out template is strictly bad that we remove it while at the same time asking if we should set up sentry which is exactly doing that just in a more sophisticated way?

@oliviertassinari
Copy link
Member Author

I do think that an already filled template is better from an effectiveness perspective. I don't think that we should spend time on it from an efficiency perspective. It's about doing the minimum of work we can get away with.

@eps1lon
Copy link
Member

eps1lon commented Aug 6, 2019

I don't think that we should spend time on it from an efficiency perspective.

Time is already spent. Now you're spending time making the spent time useless.

Edit:

I don't think that we should spend time on it from an efficiency perspective.

-- #16871 (comment)

What do you think of using Sentry?

-- #16746 (comment)

If you don't think it's worth spending time why start a discussion about more sophisticated tools?

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 6, 2019

Does it take into account the time of maintaining the logic?

It's a great case to align /discussion how we should solve problems.
In this case, I want to emphasize on the "saving work" aspect, when should we not solve problems?

@eps1lon
Copy link
Member

eps1lon commented Aug 6, 2019

Does it take into account the time of maintaining the logic?

Should I remind you of that every time we tweak the issue template? Since it's not worth our time why not leave it blank since this is all not worth our time.

@oliviertassinari
Copy link
Member Author

It don't fully follow. The issue templates are filled 50 times? a week. How many time would this one be used?

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 6, 2019

How many time would this one be used?

We don't know. The idea is to assume it's rare and to get a chance to have people use the link. We can learn from that usage. If we see that a prefilled template would save us time, then we can consider it.
I have looked at the "expand" event analytics, the least used demo is pages/system/basics/JSS.js, people expand it 2 times a day. The median used demo is expended 20 times a day. It makes me believe that we have strong test coverage, running every day.

If you don't think it's worth spending time why start a discussion about more sophisticated tools?

I'm interested in the discussion because my current point of view might be wrong? I believe that Sentry takes 10 minutes to set up and will give a full overview of the situation. I have been wondering about it for months. So far, the reasoning for not adding it was: 1. It's better to let our users filter the noise, if somebody takes the time to report something, it's probably because it's important to him. 2. We wouldn't have the time to handle the messages. 3. It would slow down the pages.

@eps1lon
Copy link
Member

eps1lon commented Aug 8, 2019

Again why do you reject an already implemented pre-filled issue template? You have given no reason other than: there are other ways or: it might not be used to which I say: then it doesnt matter if we merge it

@oliviertassinari
Copy link
Member Author

why do you reject an already implemented

Having something already implemented shouldn't change the outcome. This advantage fades away in the long term projection.

Any logic has a maintenance overhead. It makes changes harder. I think that we reject people new feature pull request for the same reason. The bet is that there is a relatively high chance that it won't be necessary. The logic is not lost. We should be able to use it, in the future, if we need it :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Component demos crashes in IE11
5 participants