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.

Sign up for GitHub

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

Jump to bottom

fix fork errors not showing #552

Merged
merged 9 commits into from
Jan 15, 2025
Merged

fix fork errors not showing #552

merged 9 commits into from
Jan 15, 2025

Conversation

techmannih
Copy link
Contributor

@techmannih techmannih commented Jan 13, 2025 •
edited
Loading

fixes #510
/claim #510

20250113151710.mp4

@techmannih techmannih requested a review from seveibar as a code owner January 13, 2025 09:41
@algora-pbc algora-pbc bot mentioned this pull request Jan 13, 2025
Closed
imrishabh18
imrishabh18 reviewed Jan 13, 2025
View reviewed changes
Copy link
Member

@imrishabh18 imrishabh18 left a comment

Choose a reason for hiding this comment

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

Looks great! I think a test file should also be added for this

fake-snippets-api/routes/api/snippets/create.ts Outdated Show resolved Hide resolved
@techmannih
Copy link
Contributor Author

@imrishabh18, how's test failing after creating a snap shot? Any idea

imrishabh18
imrishabh18 requested changes Jan 13, 2025
View reviewed changes
playwright-tests/fork-snippet.spec.ts Outdated Show resolved Hide resolved
playwright.config.ts Outdated Show resolved Hide resolved
@seveibar seveibar changed the title fix fork implementation fix fork errors not showing Jan 13, 2025
@seveibar
Copy link
Contributor

@techmannih just gotta get your playwright tests passing

seveibar
seveibar requested changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

close

@techmannih
Copy link
Contributor Author

@seveibar done

@imrishabh18
Copy link
Member

@techmannih I remember there were test files for this change?

@techmannih
Copy link
Contributor Author

yeah @imrishabh18 , there is timeout issue reaching to more timeout, these are not able to pass to playwright test

seveibar
seveibar approved these changes Jan 15, 2025
View reviewed changes
Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

Pretty good!!!

@seveibar seveibar merged commit 4bd8a54 into tscircuit:main Jan 15, 2025
5 checks passed
@seveibar seveibar mentioned this pull request Jan 24, 2025
Merged
@techmannih techmannih deleted the fork branch January 24, 2025 20:03
@seveibar seveibar mentioned this pull request Jan 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imrishabh18 imrishabh18 imrishabh18 requested changes

@seveibar seveibar seveibar approved these changes

Assignees
No one assigned
Labels
🙋 Bounty claim
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

there is no indication that you already forked a snippet (error message needed)
3 participants
@techmannih @seveibar @imrishabh18