Skip to content

Fix LegacyProtoTypeAdapterFactoryTest bitfield regex pattern. - #3083

Merged
eamonnmcmanus merged 3 commits into
google:mainfrom
eamonnmcmanus:lptaft
Aug 11, 2026
Merged

eamonnmcmanus merged 3 commits into
google:mainfrom
eamonnmcmanus:lptaft

Conversation

@eamonnmcmanus

Copy link
Copy Markdown
Member

This will allow the test to pass even after some upcoming changes to the internal representation of Java protos.

This is internal cl/958647397.

This will allow the test to pass even after some upcoming changes to the
internal representation of Java protos.
@eamonnmcmanus
eamonnmcmanus requested a review from cpovirk August 4, 2026 00:58
json.keySet().stream()
.filter(key -> pattern.matcher(key).find())
.collect(toImmutableList());
json.keySet().removeAll(keysToRemove);

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.

Not directly related, but instead of collecting the keys in keysToRemove and then doing keySet().removeAll(keysToRemove), would it be possible to directly do keySet().removeIf(...)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I think so. Obviously if we can do streams we can do removeIf. Feel free to add that change in here, or we can make a separate PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh I got confused, I'm the one who could make this change. I think I'll defer it, though.

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.

Have created #3124 for this now.

@eamonnmcmanus
eamonnmcmanus merged commit b73771d into google:main Aug 11, 2026
21 checks passed
@eamonnmcmanus
eamonnmcmanus deleted the lptaft branch August 11, 2026 22:33
@cpovirk

cpovirk commented Sep 17, 2026

Copy link
Copy Markdown
Member

(apologies for never getting around to providing the requested review on this, especially given how trivial it was)

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