Skip to content

[SPARK-45767][CORE] Delete TimeStampedHashMap and its UT - #43633

Closed
panbingkun wants to merge 3 commits into
apache:masterfrom
panbingkun:remove_TimeStampedHashMap
Closed

[SPARK-45767][CORE] Delete TimeStampedHashMap and its UT#43633
panbingkun wants to merge 3 commits into
apache:masterfrom
panbingkun:remove_TimeStampedHashMap

Conversation

@panbingkun

@panbingkunpanbingkun commented Nov 2, 2023

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The pr aims to delete TimeStampedHashMap and its UT.

Why are the changes needed?

During Pr #43578, we found that the class TimeStampedHashMap is no longer in use. Based on the suggestion, we have removed it. #43578 (comment)

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

Was this patch authored or co-authored using generative AI tooling?

No.

@panbingkun

Copy link
Copy Markdown
ContributorAuthor

@panbingkun

Copy link
Copy Markdown
ContributorAuthor

The issue of appveyor is not related to this PR.
#43631

@LuciferYang

Copy link
Copy Markdown
Contributor

I think the description of the PR should be clearer, like when this class was introduced, why it is no longer used, instead of referencing a comments.

@panbingkun

Copy link
Copy Markdown
ContributorAuthor

I think the description of the PR should be clearer, like when this class was introduced, why it is no longer used, instead of referencing a comments.

Done.

@LuciferYangLuciferYang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1, LGTM if test pass

@dongjoon-hyundongjoon-hyun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1, LGTM.

@LuciferYang

Copy link
Copy Markdown
Contributor

Merged into master for Spark 4.0. Thanks @panbingkun@dongjoon-hyun and @beliefer

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@panbingkun@LuciferYang@beliefer@dongjoon-hyun