Skip to content

Conversation

sheetalkamat
Copy link
Member

@sheetalkamat sheetalkamat commented Aug 22, 2025

Fixes #1625

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a type inconsistency where CompileTimes was being passed by value instead of by pointer throughout the compilation pipeline. The change ensures consistent pointer semantics for the CompileTimes parameter across all compilation functions.

  • Updated function signatures to accept *CompileTimes instead of CompileTimes
  • Changed call sites to pass &compileTimes when calling functions that now expect a pointer
  • Modified struct field to store *CompileTimes instead of CompileTimes

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
internal/execute/watcher.go Updated call to pass pointer to CompileTimes
internal/execute/tsc/emit.go Changed function parameters and usage to accept CompileTimes pointer
internal/execute/tsc/compile.go Updated struct field to store CompileTimes pointer
internal/execute/tsc.go Updated function parameters and call sites to use CompileTimes pointer
internal/execute/build/buildtask.go Updated call to pass pointer to CompileTimes

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

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

These aren't modified concurrently by the build system, right?

@sheetalkamat
Copy link
Member Author

These aren't modified concurrently by the build system, right?

No, they are generated per project and then get get aggregated at the end of the build for reporting aggregate statistics

@sheetalkamat sheetalkamat added this pull request to the merge queue Aug 22, 2025
Merged via the queue into main with commit 41ced9a Aug 22, 2025
22 checks passed
@sheetalkamat sheetalkamat deleted the compileTimes branch August 22, 2025 20:07
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.

--diagnostics not reporting total time with the 0822 release

3 participants