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

Revert grid template prs #24

Merged
merged 4 commits into from
Oct 21, 2024
Merged

Conversation

jaj1014
Copy link

@jaj1014 jaj1014 commented Oct 19, 2024

In an attempt to keep our fork clean, we are going to revert the commits which brought in the original grid template fix (which contains a bug). The hope is that this will make it easier to work with a single PR for upstream and our fork which will contain a modified version of the original fix.

Reverted the following commits:
https://github.com/rrweb-io/rrweb/pull/1396/commits

Note: reverting the changeset commit wasn't working w/ git revert so I manually deleted the file in a separate commit.

@jaj1014 jaj1014 requested review from guntherjh and dkindel October 19, 2024 11:27
Copy link

@guntherjh guntherjh left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @dkindel)

@jaj1014
Copy link
Author

jaj1014 commented Oct 21, 2024

Not crazy about how many tests are failing. When we upgraded pendo-main w/ upstream master after the vite upgrade, I think we missed fixing some tests that previously used jest (and now use vi).

I can try to fix it in this PR, or I can create a separate PR to try to clean up tests... thoughts?

@guntherjh
Copy link

I'd do it in a separate PR. Let's keep these changes specific to the revert

@guntherjh guntherjh merged commit 5a0cb20 into pendo-main Oct 21, 2024
3 of 5 checks passed
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