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

docs(core): Add flush-related docstrings and comments #3789

Merged
merged 1 commit into from
Jul 9, 2021

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Jul 9, 2021

In the process of trying to solve the problem of broken nextjs API routes on Vercel, I had to dig into the flush code, to figure out if it was causing the problems we were seeing. Spoiler alert: It wasn't. But since I'd already annotated the code for myself, I figured I might as well spare the next reader of that code some time if possible.

In addition to docstrings and comments, the only changes are:

  • s/_isClientProcessing/_isClientDoneProcessing: This resolves to true when processing is done, and false if processing is still happening, which is the opposite of what the original name implied.
  • s/_processing/_numProcessing: Since it represents a number, might as well make that obvious in the name, lest the casual reader think it might be a boolean or a collection of things currently being processed.
  • s/ready/clientFinished in flush: This provides a nice parallel to transportFlushed. Also, the client may well be "ready," but since this is mostly called as part of a shutdown procedure, the fact that it's ready matters less than the fact that it's finished with the work it was doing.

@lobsterkatie lobsterkatie requested a review from kamilogorek as a code owner July 9, 2021 07:28
@github-actions
Copy link
Contributor

github-actions bot commented Jul 9, 2021

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 21.43 KB (+0.01% 🔺)
@sentry/browser - Webpack 22.46 KB (+0.03% 🔺)
@sentry/react - Webpack 22.49 KB (+0.03% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 28.93 KB (+0.01% 🔺)

@lobsterkatie lobsterkatie merged commit f964a69 into master Jul 9, 2021
@lobsterkatie lobsterkatie deleted the kmclb-flush-and-drain-comments branch July 9, 2021 15:14
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.

3 participants