Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

sg: skip honey event duration if event is nil#62476

Merged
burmudar merged 4 commits into
mainfrom
wb/sg/lint-nil-panic
May 7, 2024
Merged

sg: skip honey event duration if event is nil#62476
burmudar merged 4 commits into
mainfrom
wb/sg/lint-nil-panic

Conversation

@burmudar

@burmudar burmudar commented May 7, 2024

Copy link
Copy Markdown
Contributor

runScripSerialized worked fine if you ran pnpm dedupe because it was 'instrumented' with timeCheck but if you didn't and executed runScriptSerialized without timeCheck the context won't be Honeycomb enriched and thus the event will be nil and then the whole method falls apart.

Previously this also just checked the locking duration of the mutex 🤔 and I assume it wanted to check the duration of the pnpm dedupe?

Either way, it now checks if the event is nil before using it.

I think this addresses #62471 and https://github.com/sourcegraph/devx-support/issues/933

Test plan

Tested locally

@burmudar burmudar requested review from Strum355 and jhchabran May 7, 2024 08:00
@cla-bot cla-bot Bot added the cla-signed label May 7, 2024
Comment thread dev/sg/linters/linters.go Outdated
@Strum355

Strum355 commented May 7, 2024

Copy link
Copy Markdown
Contributor

Not sure if we even still need any of this 🤔 we could maybe delete this as we dont check honeycomb stats for sg lint anymore

@burmudar

burmudar commented May 7, 2024

Copy link
Copy Markdown
Contributor Author

Not sure if we even still need any of this 🤔 we could maybe delete this as we dont check honeycomb stats for sg lint anymore

yeah then I'll remove it

@burmudar burmudar requested a review from Strum355 May 7, 2024 14:23
Comment thread dev/sg/linters/linters.go Outdated
@burmudar burmudar force-pushed the wb/sg/lint-nil-panic branch from 8f74f0e to 61700dd Compare May 7, 2024 14:25
@burmudar burmudar self-assigned this May 7, 2024
Comment thread dev/sg/linters/linters.go Outdated
Co-authored-by: Noah S-C <noah@sourcegraph.com>
@burmudar burmudar merged commit bd7d44b into main May 7, 2024
@burmudar burmudar deleted the wb/sg/lint-nil-panic branch May 7, 2024 15:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants