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

Remove rpc/js as it is now in TypeScript SDK #365

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Remove rpc/js as it is now in TypeScript SDK #365

merged 1 commit into from
Oct 8, 2024

Conversation

edaniels
Copy link
Contributor

@edaniels edaniels commented Oct 7, 2024

@maximpertsov I'll defer to you and team on deleting this e2e test here. I don't know how much value it servers compared to someone writing a viam machine specific version in TS-SDK.

@viambot viambot added the safe to test Mark as safe to test label Oct 7, 2024
@viambot viambot added safe to test Mark as safe to test and removed safe to test Mark as safe to test labels Oct 7, 2024
@edaniels
Copy link
Contributor Author

edaniels commented Oct 7, 2024

Tests wont pass since workflow is running off of main. But we're just removing things here. There's no more js and just all go(utils)

@maximpertsov
Copy link
Contributor

maximpertsov commented Oct 8, 2024

I'll defer to you and team on deleting this e2e test here. I don't know how much value it servers compared to someone writing a viam machine specific version in TS-SDK.

I added that E2E test because the JS rpc code didn't have any test coverage before. You added some tests when you migrated to connect-es, right? If so then I think we can just rely on those and delete the existing E2E test.

Sorry! I made this comment before looking at the diff - yeah since we are removing rpc/js entirely then that E2E test is pointless anyway.

@edaniels
Copy link
Contributor Author

edaniels commented Oct 8, 2024

I'd say webrtc is probably not covered now in the TS SDK without an E2E test. But there may be one in app to pull over

@edaniels edaniels merged commit 9fe6e1e into main Oct 8, 2024
4 of 8 checks passed
@edaniels edaniels deleted the rmrpcjs branch October 8, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test Mark as safe to test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants