Skip to content

Conversation

@brandon-pereira
Copy link
Member

Noticed some errors during runtime testing of another PR that should have been caught by typescript. This fixes reduces the chances of that slipping through CI

@brandon-pereira brandon-pereira requested review from a team and pulpdrew and removed request for a team December 29, 2025 16:34
@changeset-bot
Copy link

changeset-bot bot commented Dec 29, 2025

⚠️ No Changeset found

Latest commit: a49d2a2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Dec 29, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Dec 29, 2025 5:15pm

@claude
Copy link

claude bot commented Dec 29, 2025

Code Review

No critical issues found.

Summary

This PR successfully removes @ts-nocheck directives from 4 frontend files and adds proper TypeScript typing. The changes are well-structured and improve type safety:

  • AuthPage.tsx: Proper HTTPError type checking added, unused import removed
  • LandingHeader.tsx: Clean @ts-nocheck removal
  • Spotlights.tsx: Correctly uses id property (from SavedSearch schema) instead of _id, unused imports removed
  • api.ts: Proper type annotations added to useDashboards and useRegisterPassword

All type changes align with existing schemas (SavedSearchSchema uses id, Dashboard type uses id). No runtime behavior changes.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 29, 2025

E2E Test Results

All tests passed • 50 passed • 4 skipped • 697s

Status Count
✅ Passed 50
❌ Failed 0
⚠️ Flaky 1
⏭️ Skipped 4

Tests ran across 4 shards in parallel.

View full report →

Copy link
Contributor

@pulpdrew pulpdrew left a comment

Choose a reason for hiding this comment

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

LGTM

@kodiakhq kodiakhq bot merged commit 3019bec into main Dec 29, 2025
12 checks passed
@kodiakhq kodiakhq bot deleted the brandon/remove-ts-nochecks branch December 29, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants