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

Update Analysis API to 2.0.20-dev-6911 #3652

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

vmishenev
Copy link
Member

List of changes:

  • KT-68462
  • partially KT-68884 and others

List of changes:
- KT-68462
- partially KT-68884
and others
Copy link
Collaborator

@whyoleg whyoleg left a comment

Choose a reason for hiding this comment

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

Overall, nice!
Still, there are two things I want to resolve:

  1. consistent naming: some declarations in AA changed their names, and so some of our functions are also changed, but others don't. I do have two suggestions here, it would be nice to consistently use one of them:
  • we change everything to align with the new naming
  • we change nothing until AA is stable enough and no renaming is expected
  1. using of sequences: AFAIU AA almost all the time returns sequences, and in our case we need to handle most of the declarations from those sequences. I believe we should convert them to lists as soon as possible to not stick in a situation that we recreate those sequences on every call. This was the issue which I fixed not so long ago which causes big performance degradation in K2 analysis.

@vmishenev
Copy link
Member Author

consistent naming: some declarations in AA changed their names, and so some of our functions are also changed, but others don't. I do have two suggestions here, it would be nice to consistently use one of them:

we change everything to align with the new naming
we change nothing until AA is stable enough and no renaming is expected

I am trying to do the former to avoid forgotten context.
But renaming everything is quite hard since you should it manually.
For example, I renamed only namedClassOrObjectSymbol to namedClassSymbol since here the name is crucial and scared for me.

My approach is

  • rename only important changes;
  • after the AA stabilization, we can rename the remains if it makes sense

I believe we should convert them to lists as soon as possible to not stick in a situation that we recreate those sequences on every call.

I agree, it is a good point. Using sequences from AA should be moderated.
I would say - in the end of a single chain.

@vmishenev vmishenev force-pushed the vmishenev/Update-Analysis-API-to-2.0.20-dev-6911 branch from a5e1351 to ef9bf7f Compare June 25, 2024 07:31
@vmishenev vmishenev requested a review from whyoleg June 25, 2024 08:03
@vmishenev vmishenev merged commit 75fd544 into master Jun 25, 2024
14 checks passed
@vmishenev vmishenev deleted the vmishenev/Update-Analysis-API-to-2.0.20-dev-6911 branch June 25, 2024 16:32
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.

2 participants