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

Deprecate GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean) #796

Merged
merged 27 commits into from
Jul 10, 2024

Conversation

Ao-senXiong
Copy link
Member

@Ao-senXiong Ao-senXiong commented Jun 26, 2024

Cherry-pick from opprop#215, we can close that PR after merge this one.

Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

I still like this change, I think removing the extra parameter is good.
Can you add a changelog and also see whether the method is mentioned in the manual?

@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Jun 27, 2024
@Ao-senXiong
Copy link
Member Author

I still like this change, I think removing the extra parameter is good. Can you add a changelog and also see whether the method is mentioned in the manual?

Added a changelog entry and remove document no longer useful in the manual.

@Ao-senXiong Ao-senXiong requested a review from wmdietl June 27, 2024 18:50
@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Jun 27, 2024
@wmdietl wmdietl changed the title Remove useFlow params Remove GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean) Jun 28, 2024
Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

Thanks!

@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Jun 28, 2024
docs/CHANGELOG.md Outdated Show resolved Hide resolved
@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Jul 7, 2024
Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -5,6 +5,10 @@ Version 3.42.0-eisop4 (April ?, 2024)

**Implementation details:**

Deprecated the `GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean)` overload.
Instead, set `GenericAnnotatedTypeFactory#useFlow` appropriately and
Copy link
Member

Choose a reason for hiding this comment

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

I also see deletions in creating-a-checker.tex, no additions. Did you forget to add your changes in the manual?

docs/CHANGELOG.md Outdated Show resolved Hide resolved
docs/CHANGELOG.md Outdated Show resolved Hide resolved
@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Jul 7, 2024
@Ao-senXiong
Copy link
Member Author

I also see deletions in creating-a-checker.tex, no additions. Did you forget to add your changes in the manual?

I forget to commit my changes. Now it is there.

@Ao-senXiong Ao-senXiong requested a review from wmdietl July 8, 2024 19:16
@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Jul 8, 2024
docs/CHANGELOG.md Outdated Show resolved Hide resolved
}

@Override
protected void addComputedTypeAnnotations(Tree tree, AnnotatedTypeMirror type) {
Copy link
Member

Choose a reason for hiding this comment

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

An @see to addComputedTypeAnnotationsWithoutFlow from here would be good, to connect them in both directions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated with inheritance doc and @see tag.

@@ -2014,9 +2013,35 @@ public AnnotatedTypeMirror getDefaultAnnotations(Tree tree, AnnotatedTypeMirror
* @param tree an AST node
* @param type the type obtained from tree
* @param iUseFlow whether to use information from dataflow analysis
* @deprecated use {@link #addComputedTypeAnnotations(Tree, AnnotatedTypeMirror)} instead
Copy link
Member

Choose a reason for hiding this comment

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

Also mention addComputedTypeAnnotationsWithoutFlow here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, added in new commit.

Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

Thanks, some small documentation comments.

@wmdietl wmdietl assigned Ao-senXiong and unassigned wmdietl Jul 9, 2024
@Ao-senXiong Ao-senXiong requested a review from wmdietl July 9, 2024 18:22
@Ao-senXiong Ao-senXiong assigned wmdietl and unassigned Ao-senXiong Jul 9, 2024
Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

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

Thanks!

@wmdietl wmdietl enabled auto-merge (squash) July 10, 2024 01:28
@wmdietl wmdietl changed the title Deprecated GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean) Deprecate GenericAnnotatedTypeFactory#addComputedTypeAnnotations(Tree, AnnotatedTypeMirror, boolean) Jul 10, 2024
@wmdietl wmdietl merged commit dcb31d3 into eisop:master Jul 10, 2024
38 of 39 checks passed
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