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

Cleanup of redundant HdrToken & WKS #11751

Merged
merged 1 commit into from
Sep 5, 2024
Merged

Conversation

zwoop
Copy link
Contributor

@zwoop zwoop commented Aug 29, 2024

No description provided.

@zwoop zwoop added the Cleanup label Aug 29, 2024
@zwoop zwoop added this to the 10.1.0 milestone Aug 29, 2024
@zwoop zwoop self-assigned this Aug 29, 2024
@zwoop zwoop requested a review from bneradt August 29, 2024 22:01
Copy link
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

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

This looks reasonable 👍

Probably, _hdrtoken_strs and _hdrtoken_commonly_tokenized_strs had different meanings in the past, but current code doesn't have it anymore.

Copy link
Contributor

@JosiahWI JosiahWI left a comment

Choose a reason for hiding this comment

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

Agree with Masaori. Looks good, thanks!

@zwoop
Copy link
Contributor Author

zwoop commented Aug 30, 2024

This looks reasonable 👍

Probably, _hdrtoken_strs and _hdrtoken_commonly_tokenized_strs had different meanings in the past, but current code doesn't have it anymore.

Yeh they did, _hdrtoken_commonly_tokenized_strs was smaller / different, but at some point (I can't find when / where) they were made identical. Maybe as part of us changing the hash map / searches ?

@zwoop zwoop merged commit 4ccae24 into apache:master Sep 5, 2024
15 checks passed
@zwoop zwoop deleted the CleanupHdrToken branch September 5, 2024 02:46
Sign up for free to 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