-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
refactor: importAnalysisBuild
#19723
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
base: main
Are you sure you want to change the base?
refactor: importAnalysisBuild
#19723
Conversation
154314e
to
4098d6f
Compare
e5912cc
to
0d85f7d
Compare
309a0f4
to
aaa2273
Compare
62ce4aa
to
0325471
Compare
0958fd6
to
66cd77e
Compare
With this refactor, I also have a follow up perf optimization to de-duplicate the chunk registry: |
66cd77e
to
2496007
Compare
@sapphi-red Any updates on this? Just so you know, this is a refactor as part of a larger fix in #19722. Would love to get these in before it becomes outdated. |
We normally require 2 or more reviews to merge a PR, so another review is needed. Let me add this to the team board so that I can discuss in the team meeting and speed up the process. |
First of all, thank you for your work on these pull requests. In our last team meeting, we discussed refactoring PRs and established a policy for coding style changes (see contribution guide change). Even if we also agree some of these PRs have value, they also make git history more difficult to navigate and potentially introduce subtle issues. In general, we'd like to only merge refactors when they are required to implement features or bug fixes. So we prefer not to merge this specific PR as it primarily contains style adjustments. However, we'd still love to merge the other dependent PRs if you'd like to rebase those onto main. Thanks! |
Description
Pulling out some minor refactors from #19722