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

chore: delete embedded trace viewer #555

Merged
merged 3 commits into from
Nov 18, 2024

Conversation

Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Nov 14, 2024

We decided not to ship the embedded trace viewer. I have some changes to the trace viewer that broke the tests for it, so this PR removes the code altogether.

@Skn0tt Skn0tt self-assigned this Nov 14, 2024
@Skn0tt Skn0tt requested a review from dgozman November 18, 2024 13:14
@Skn0tt
Copy link
Member Author

Skn0tt commented Nov 18, 2024

cc @ruifigueira. We're dropping your embedded trace viewer feature for maintenance reasons. It's yet another distribution of the trace viewer, and we already have to support so many of them (UI mode, local trace viewer, hosted trace viewer, ...). Adding one more makes it harder for us to maintain, and it seems like the product benefit doesn't offset that. We kept the code for it the past weeks to see if we feel different, but it now started failing some tests for changes we'd like to make.

So we've made the decision to revert - not because of the quality of your work, but because of maintenance overhead. Hope that's OK for you - you did great work on this, and we're more than open to future contributions of yours.

@Skn0tt Skn0tt merged commit facbc06 into microsoft:main Nov 18, 2024
6 checks passed
@ruifigueira
Copy link
Contributor

I was a victim of my own code:

microsoft/playwright#33643 (comment)

It's now time to let it go (well, sort of, because git doesn't let, this is probably the equivalent to purgatory).

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.

3 participants