solved #274 review youtube embed handling #275
Conversation
|
can anyone please tell me what is the problem? Why the tests are failing? |
|
Hey @shibasisp, that was quick! Fantastic! Guess what? I totally forgot to list that we have unit tests around this as well, and for TravisCI to pass the PR, the youtube unit tests need to be removed as well. Oops. Those are in the However, @ashleygwilliams brought up a good point in #274, that the ideal outcome here might be different than just removing the youtube stuff, because there are definitely users (the npm site, at least) that need it. So if you don't mind, I have to hang on to this until she's able to investigate a bit more and we can decide where the youtube processing stuff needs to live. |
|
Ok fine.. |
|
hi! so yes- until i can completely move the npm docs off of marky-markdown (it's in the plan, just not a priority at the moment) we'll need to keep this functionality. that being said, because it is not inline with the product direction of parity with github-flavored markdown, i would be happy to see it moved behind a flag as part of the deprecation path! cc/@revin |
|
That sounds good. So @shibasisp does that sound interesting to you? Basically add an option in index.js, say, Then instead of removing the tests, they'd need updated so the current tests pass if the option is set to Is that enough information for you to know what changes to make? I'm totally happy to help if anything is confusing. 👍 |
|
no preference on name for the flag! |
|
Hey again @shibasisp, do you have the time/availability to implement the version of this where it's deprecated and not removed entirely? If not, I can take over from here if you like. No pressure. Thanks again! |
is this okay?